From cf79ced740cb1c5f3497004bb996224ca86713aa Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 9 Aug 2021 18:18:12 -0400 Subject: [PATCH 1/6] Rework how strategy constants are identified Up to this point, we've had to rely on making `Strategy` constants private to ensure that the only available constants are strategies. With the current setup, the existence of a constant that's not a strategy would break `Strategy#strategies` and `Livecheck#livecheck_strategy_names`. Instead, we can achieve the same goal by skipping over constants that aren't a class. Other than saving us from having to make these constants private, this is necessary to be able to create a `Strategy` constant that can be used in all strategies. --- Library/Homebrew/livecheck/livecheck.rb | 8 +++++--- Library/Homebrew/livecheck/strategy.rb | 17 ++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/Library/Homebrew/livecheck/livecheck.rb b/Library/Homebrew/livecheck/livecheck.rb index 390d83817d..e321fb8895 100644 --- a/Library/Homebrew/livecheck/livecheck.rb +++ b/Library/Homebrew/livecheck/livecheck.rb @@ -56,9 +56,11 @@ module Homebrew # Cache demodulized strategy names, to avoid repeating this work @livecheck_strategy_names = {} - Strategy.constants.sort.each do |strategy_symbol| - strategy = Strategy.const_get(strategy_symbol) - @livecheck_strategy_names[strategy] = strategy.name.demodulize + Strategy.constants.sort.each do |const_symbol| + constant = Strategy.const_get(const_symbol) + next unless constant.is_a?(Class) + + @livecheck_strategy_names[constant] = T.must(constant.name).demodulize end @livecheck_strategy_names.freeze end diff --git a/Library/Homebrew/livecheck/strategy.rb b/Library/Homebrew/livecheck/strategy.rb index 05229ea1b0..cb27336dd9 100644 --- a/Library/Homebrew/livecheck/strategy.rb +++ b/Library/Homebrew/livecheck/strategy.rb @@ -75,13 +75,6 @@ module Homebrew # In rare cases, this can also be a double newline (`\n\n`). HTTP_HEAD_BODY_SEPARATOR = "\r\n\r\n" - # The `#strategies` method expects `Strategy` constants to be strategies, - # so constants we create need to be private for this to work properly. - private_constant :DEFAULT_PRIORITY, :CURL_CONNECT_TIMEOUT, :CURL_MAX_TIME, - :CURL_PROCESS_TIMEOUT, :DEFAULT_CURL_ARGS, - :PAGE_HEADERS_CURL_ARGS, :PAGE_CONTENT_CURL_ARGS, - :DEFAULT_CURL_OPTIONS, :HTTP_HEAD_BODY_SEPARATOR - # Creates and/or returns a `@strategies` `Hash`, which maps a snake # case strategy name symbol (e.g. `:page_match`) to the associated # {Strategy}. @@ -93,10 +86,12 @@ module Homebrew return @strategies if defined? @strategies @strategies = {} - constants.sort.each do |strategy_symbol| - key = strategy_symbol.to_s.underscore.to_sym - strategy = const_get(strategy_symbol) - @strategies[key] = strategy + Strategy.constants.sort.each do |const_symbol| + constant = Strategy.const_get(const_symbol) + next unless constant.is_a?(Class) + + key = const_symbol.to_s.underscore.to_sym + @strategies[key] = constant end @strategies end From 56dd89114de881a223c4b5ed79b041ce8902b929 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Tue, 10 Aug 2021 11:09:55 -0400 Subject: [PATCH 2/6] Standardize valid strategy block return types Valid `strategy` block return types currently vary between strategies. Some only accept a string whereas others accept a string or array of strings. [`strategy` blocks also accept a `nil` return (to simplify early returns) but this was already standardized across strategies.] While some strategies only identify one version by default (where a string is an appropriate return type), it could be that a strategy block identifies more than one version. In this situation, the strategy would need to be modified to accept (and work with) an array from a `strategy` block. Rather than waiting for this to become a problem, this modifies all strategies to standardize on allowing `strategy` blocks to return a string or array of strings (even if only one of these is currently used in practice). Standardizing valid return types helps to further simplify the mental model for `strategy` blocks and reduce cognitive load. This commit extracts related logic from `#find_versions` into methods like `#versions_from_content`, which is conceptually similar to `PageMatch#page_matches` (renamed to `#versions_from_content` for consistency). This allows us to write tests for the related code without having to make network requests (or stub them) at this point. In general, this also helps to better align the structure of strategies and how the various `#find_versions` methods work with versions. There's still more planned work to be done here but this is a step in the right direction. --- Library/Homebrew/livecheck/strategy.rb | 23 +++++ .../livecheck/strategy/electron_builder.rb | 35 +++---- .../livecheck/strategy/extract_plist.rb | 46 +++++---- Library/Homebrew/livecheck/strategy/git.rb | 97 +++++++++++-------- .../livecheck/strategy/header_match.rb | 69 +++++++------ .../Homebrew/livecheck/strategy/page_match.rb | 17 +--- .../Homebrew/livecheck/strategy/sparkle.rb | 37 ++++--- .../strategy/electron_builder_spec.rb | 40 ++++---- .../livecheck/strategy/extract_plist_spec.rb | 72 ++++++++++++++ .../test/livecheck/strategy/git_spec.rb | 70 ++++++++++++- .../livecheck/strategy/header_match_spec.rb | 97 ++++++++++++++++++- .../livecheck/strategy/page_match_spec.rb | 41 +++++--- .../test/livecheck/strategy/sparkle_spec.rb | 45 ++++++++- .../Homebrew/test/livecheck/strategy_spec.rb | 16 +++ 14 files changed, 534 insertions(+), 171 deletions(-) create mode 100644 Library/Homebrew/test/livecheck/strategy/extract_plist_spec.rb diff --git a/Library/Homebrew/livecheck/strategy.rb b/Library/Homebrew/livecheck/strategy.rb index cb27336dd9..0040bc4821 100644 --- a/Library/Homebrew/livecheck/strategy.rb +++ b/Library/Homebrew/livecheck/strategy.rb @@ -75,6 +75,10 @@ module Homebrew # In rare cases, this can also be a double newline (`\n\n`). HTTP_HEAD_BODY_SEPARATOR = "\r\n\r\n" + # An error message to use when a `strategy` block returns a value of + # an inappropriate type. + INVALID_BLOCK_RETURN_VALUE_MSG = "Return value of a strategy block must be a string or array of strings." + # Creates and/or returns a `@strategies` `Hash`, which maps a snake # case strategy name symbol (e.g. `:page_match`) to the associated # {Strategy}. @@ -218,6 +222,25 @@ module Homebrew messages: [error_msg.presence || "cURL failed without an error"], } end + + # Handles the return value from a `strategy` block in a `livecheck` + # block. + # + # @param value [] the return value from a `strategy` block + # @return [Array] + sig { params(value: T.untyped).returns(T::Array[String]) } + def self.handle_block_return(value) + case value + when String + [value] + when Array + value.compact.uniq + when nil + [] + else + raise TypeError, INVALID_BLOCK_RETURN_VALUE_MSG + end + end end end end diff --git a/Library/Homebrew/livecheck/strategy/electron_builder.rb b/Library/Homebrew/livecheck/strategy/electron_builder.rb index 45626f315d..5c27824235 100644 --- a/Library/Homebrew/livecheck/strategy/electron_builder.rb +++ b/Library/Homebrew/livecheck/strategy/electron_builder.rb @@ -30,34 +30,28 @@ module Homebrew URL_MATCH_REGEX.match?(url) end - # Extract version information from page content. + # Parses YAML text and identifies versions in it. # - # @param content [String] the content to check - # @return [String] + # @param content [String] the YAML text to parse and check + # @return [Array] sig { params( content: String, - block: T.nilable(T.proc.params(arg0: T::Hash[String, T.untyped]).returns(T.nilable(String))), - ).returns(T.nilable(String)) + block: T.nilable( + T.proc.params(arg0: T::Hash[String, T.untyped]).returns(T.any(String, T::Array[String], NilClass)), + ), + ).returns(T::Array[String]) } - def self.version_from_content(content, &block) + def self.versions_from_content(content, &block) require "yaml" yaml = YAML.safe_load(content) - return if yaml.blank? + return [] if yaml.blank? - if block - case (value = block.call(yaml)) - when String - return value - when nil - return - else - raise TypeError, "Return value of `strategy :electron_builder` block must be a string." - end - end + return Strategy.handle_block_return(block.call(yaml)) if block - yaml["version"] + version = yaml["version"] + version.present? ? [version] : [] end # Checks the content at the URL for new versions. @@ -81,8 +75,9 @@ module Homebrew match_data.merge!(Strategy.page_content(url)) content = match_data.delete(:content) - version = version_from_content(content, &block) - match_data[:matches][version] = Version.new(version) if version + versions_from_content(content, &block).each do |version_text| + match_data[:matches][version_text] = Version.new(version_text) + end match_data end diff --git a/Library/Homebrew/livecheck/strategy/extract_plist.rb b/Library/Homebrew/livecheck/strategy/extract_plist.rb index 46b7d12e08..c9a12b0ce6 100644 --- a/Library/Homebrew/livecheck/strategy/extract_plist.rb +++ b/Library/Homebrew/livecheck/strategy/extract_plist.rb @@ -50,13 +50,37 @@ module Homebrew delegate short_version: :bundle_version end - # Checks the content at the URL for new versions. + # Identify versions from `Item`s produced using + # {UnversionedCaskChecker} version information. + # + # @param items [Hash] a hash of `Item`s containing version information + # @return [Array] + sig { + params( + items: T::Hash[String, Item], + block: T.nilable( + T.proc.params(arg0: T::Hash[String, Item]).returns(T.any(String, T::Array[String], NilClass)), + ), + ).returns(T::Array[String]) + } + def self.versions_from_items(items, &block) + return Strategy.handle_block_return(block.call(items)) if block + + items.map do |_key, item| + item.bundle_version.nice_version + end.compact.uniq + end + + # Uses {UnversionedCaskChecker} on the provided cask to identify + # versions from `plist` files. sig { params( url: String, regex: T.nilable(Regexp), cask: Cask::Cask, - block: T.nilable(T.proc.params(arg0: T::Hash[String, Item]).returns(T.nilable(String))), + block: T.nilable( + T.proc.params(arg0: T::Hash[String, Item]).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask:, &block) @@ -66,22 +90,10 @@ module Homebrew match_data = { matches: {}, regex: regex, url: url } unversioned_cask_checker = UnversionedCaskChecker.new(cask) - versions = unversioned_cask_checker.all_versions.transform_values { |v| Item.new(bundle_version: v) } + items = unversioned_cask_checker.all_versions.transform_values { |v| Item.new(bundle_version: v) } - if block - case (value = block.call(versions)) - when String - match_data[:matches][value] = Version.new(value) - when nil - return match_data - else - raise TypeError, "Return value of `strategy :extract_plist` block must be a string." - end - elsif versions.any? - versions.each_value do |item| - version = item.bundle_version.nice_version - match_data[:matches][version] = Version.new(version) - end + versions_from_items(items, &block).each do |version_text| + match_data[:matches][version_text] = Version.new(version_text) end match_data diff --git a/Library/Homebrew/livecheck/strategy/git.rb b/Library/Homebrew/livecheck/strategy/git.rb index 50a0e509a2..55c3c05562 100644 --- a/Library/Homebrew/livecheck/strategy/git.rb +++ b/Library/Homebrew/livecheck/strategy/git.rb @@ -30,6 +30,19 @@ module Homebrew # lowest to highest). PRIORITY = 8 + # The default regex used to naively identify numeric versions from tags + # when a regex isn't provided. + DEFAULT_REGEX = /\D*(.+)/.freeze + + # Whether the strategy can be applied to the provided URL. + # + # @param url [String] the URL to match against + # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } + def self.match?(url) + (DownloadStrategyDetector.detect(url) <= GitDownloadStrategy) == true + end + # Fetches a remote Git repository's tags using `git ls-remote --tags` # and parses the command's output. If a regex is provided, it will be # used to filter out any tags that don't match it. @@ -61,12 +74,42 @@ module Homebrew tags_data end - # Whether the strategy can be applied to the provided URL. + # Identify versions from tag strings using a provided regex or the + # `DEFAULT_REGEX`. The regex is expected to use a capture group around + # the version text. # - # @param url [String] the URL to match against - # @return [Boolean] - def self.match?(url) - (DownloadStrategyDetector.detect(url) <= GitDownloadStrategy) == true + # @param tags [Array] the tags to identify versions from + # @param regex [Regexp, nil] a regex to identify versions + # @return [Array] + sig { + params( + tags: T::Array[String], + regex: T.nilable(Regexp), + block: T.nilable( + T.proc.params(arg0: T::Array[String], arg1: T.nilable(Regexp)) + .returns(T.any(String, T::Array[String], NilClass)), + ), + ).returns(T::Array[String]) + } + def self.versions_from_tags(tags, regex = nil, &block) + return Strategy.handle_block_return(block.call(tags, regex || DEFAULT_REGEX)) if block + + tags_only_debian = tags.all? { |tag| tag.start_with?("debian/") } + + tags.map do |tag| + # Skip tag if it has a 'debian/' prefix and upstream does not do + # only 'debian/' prefixed tags + next if tag =~ %r{^debian/} && !tags_only_debian + + if regex + # Use the first capture group (the version) + tag.scan(regex).first&.first + else + # Remove non-digits from the start of the tag and use that as the + # version text + tag[DEFAULT_REGEX, 1] + end + end.compact.uniq end # Checks the Git tags for new versions. When a regex isn't provided, @@ -82,54 +125,26 @@ module Homebrew regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), block: T.nilable( - T.proc.params(arg0: T::Array[String]).returns(T.any(String, T::Array[String], NilClass)), + T.proc.params(arg0: T::Array[String], arg1: T.nilable(Regexp)) + .returns(T.any(String, T::Array[String], NilClass)), ), ).returns(T::Hash[Symbol, T.untyped]) } - def self.find_versions(url, regex, cask: nil, &block) + def self.find_versions(url, regex = nil, cask: nil, &block) match_data = { matches: {}, regex: regex, url: url } tags_data = tag_info(url, regex) + tags = tags_data[:tags] if tags_data.key?(:messages) match_data[:messages] = tags_data[:messages] - return match_data if tags_data[:tags].blank? + return match_data if tags.blank? end - tags_only_debian = tags_data[:tags].all? { |tag| tag.start_with?("debian/") } - - if block - case (value = block.call(tags_data[:tags], regex)) - when String - match_data[:matches][value] = Version.new(value) - when Array - value.compact.uniq.each do |tag| - match_data[:matches][tag] = Version.new(tag) - end - when nil - return match_data - else - raise TypeError, "Return value of `strategy :git` block must be a string or array of strings." - end - - return match_data - end - - tags_data[:tags].each do |tag| - # Skip tag if it has a 'debian/' prefix and upstream does not do - # only 'debian/' prefixed tags - next if tag =~ %r{^debian/} && !tags_only_debian - - captures = regex.is_a?(Regexp) ? tag.scan(regex) : [] - tag_cleaned = if captures[0].is_a?(Array) - captures[0][0] # Use the first capture group (the version) - else - tag[/\D*(.*)/, 1] # Remove non-digits from the start of the tag - end - - match_data[:matches][tag] = Version.new(tag_cleaned) + versions_from_tags(tags, regex, &block).each do |version_text| + match_data[:matches][version_text] = Version.new(version_text) rescue TypeError - nil + next end match_data diff --git a/Library/Homebrew/livecheck/strategy/header_match.rb b/Library/Homebrew/livecheck/strategy/header_match.rb index af93c1f34d..66d7bccfbe 100644 --- a/Library/Homebrew/livecheck/strategy/header_match.rb +++ b/Library/Homebrew/livecheck/strategy/header_match.rb @@ -1,4 +1,4 @@ -# typed: false +# typed: true # frozen_string_literal: true require_relative "page_match" @@ -36,6 +36,39 @@ module Homebrew URL_MATCH_REGEX.match?(url) end + # Identify versions from HTTP headers. + # + # @param headers [Hash] a hash of HTTP headers to check for versions + # @param regex [Regexp, nil] a regex to use to identify versions + # @return [Array] + sig { + params( + headers: T::Hash[String, String], + regex: T.nilable(Regexp), + block: T.nilable( + T.proc.params( + arg0: T::Hash[String, String], + arg1: T.nilable(Regexp), + ).returns(T.any(String, T::Array[String], NilClass)), + ), + ).returns(T::Array[String]) + } + def self.versions_from_headers(headers, regex = nil, &block) + return Strategy.handle_block_return(block.call(headers, regex)) if block + + DEFAULT_HEADERS_TO_CHECK.map do |header_name| + header_value = headers[header_name] + next if header_value.blank? + + if regex + header_value[regex, 1] + else + v = Version.parse(header_value, detected_from_url: true) + v.null? ? nil : v.to_s + end + end.compact.uniq + end + # Checks the final URL for new versions after following all redirections, # using the provided regex for matching. sig { @@ -43,7 +76,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: T::Hash[String, String]).returns(T.nilable(String))), + block: T.nilable( + T.proc.params(arg0: T::Hash[String, String], arg1: T.nilable(Regexp)).returns(T.nilable(String)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) @@ -53,36 +88,12 @@ module Homebrew # Merge the headers from all responses into one hash merged_headers = headers.reduce(&:merge) + return match_data if merged_headers.blank? - version = if block - case (value = block.call(merged_headers, regex)) - when String - value - when nil - return match_data - else - raise TypeError, "Return value of `strategy :header_match` block must be a string." - end - else - value = nil - DEFAULT_HEADERS_TO_CHECK.each do |header_name| - header_value = merged_headers[header_name] - next if header_value.blank? - - if regex - value = header_value[regex, 1] - else - v = Version.parse(header_value, detected_from_url: true) - value = v.to_s unless v.null? - end - break if value.present? - end - - value + versions_from_headers(merged_headers, regex, &block).each do |version_text| + match_data[:matches][version_text] = Version.new(version_text) end - match_data[:matches][version] = Version.new(version) if version - match_data end end diff --git a/Library/Homebrew/livecheck/strategy/page_match.rb b/Library/Homebrew/livecheck/strategy/page_match.rb index b71ca9e9e7..873d54a4fa 100644 --- a/Library/Homebrew/livecheck/strategy/page_match.rb +++ b/Library/Homebrew/livecheck/strategy/page_match.rb @@ -54,19 +54,8 @@ module Homebrew ), ).returns(T::Array[String]) } - def self.page_matches(content, regex, &block) - if block - case (value = block.call(content, regex)) - when String - return [value] - when Array - return value.compact.uniq - when nil - return [] - else - raise TypeError, "Return value of `strategy :page_match` block must be a string or array of strings." - end - end + def self.versions_from_content(content, regex, &block) + return Strategy.handle_block_return(block.call(content, regex)) if block content.scan(regex).map do |match| case match @@ -109,7 +98,7 @@ module Homebrew end return match_data if content.blank? - page_matches(content, regex, &block).each do |match_text| + versions_from_content(content, regex, &block).each do |match_text| match_data[:matches][match_text] = Version.new(match_text) end diff --git a/Library/Homebrew/livecheck/strategy/sparkle.rb b/Library/Homebrew/livecheck/strategy/sparkle.rb index a34802405e..9e7cf4f1cf 100644 --- a/Library/Homebrew/livecheck/strategy/sparkle.rb +++ b/Library/Homebrew/livecheck/strategy/sparkle.rb @@ -138,6 +138,26 @@ module Homebrew items.max_by { |item| [item.pub_date, item.bundle_version] } end + # Identify versions from content + # + # @param content [String] the content to pull version information from + # @return [Array] + sig { + params( + content: String, + block: T.nilable(T.proc.params(arg0: Item).returns(T.any(String, T::Array[String], NilClass))), + ).returns(T::Array[String]) + } + def self.versions_from_content(content, &block) + item = item_from_content(content) + return [] if item.blank? + + return Strategy.handle_block_return(block.call(item)) if block + + version = item.bundle_version&.nice_version + version.present? ? [version] : [] + end + # Checks the content at the URL for new versions. sig { params( @@ -155,21 +175,8 @@ module Homebrew match_data.merge!(Strategy.page_content(url)) content = match_data.delete(:content) - if (item = item_from_content(content)) - version = if block - case (value = block.call(item)) - when String - value - when nil - return match_data - else - raise TypeError, "Return value of `strategy :sparkle` block must be a string." - end - else - item.bundle_version&.nice_version - end - - match_data[:matches][version] = Version.new(version) if version + versions_from_content(content, &block).each do |version_text| + match_data[:matches][version_text] = Version.new(version_text) end match_data diff --git a/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb b/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb index 43ac739c31..d76f53d670 100644 --- a/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb @@ -1,7 +1,7 @@ # typed: false # frozen_string_literal: true -require "livecheck/strategy/electron_builder" +require "livecheck/strategy" describe Homebrew::Livecheck::Strategy::ElectronBuilder do subject(:electron_builder) { described_class } @@ -26,6 +26,8 @@ describe Homebrew::Livecheck::Strategy::ElectronBuilder do EOS } + let(:versions) { ["1.2.3"] } + describe "::match?" do it "returns true for any URL pointing to a YAML file" do expect(electron_builder.match?(valid_url)).to be true @@ -36,32 +38,34 @@ describe Homebrew::Livecheck::Strategy::ElectronBuilder do end end - describe "::version_from_content" do - let(:version_from_electron_builder_yaml) { electron_builder.version_from_content(electron_builder_yaml) } - - it "returns nil if content is blank" do - expect(electron_builder.version_from_content("")).to be nil + describe "::versions_from_content" do + it "returns an empty array if content is blank" do + expect(electron_builder.versions_from_content("")).to eq([]) end - it "returns a version string when given YAML data" do - expect(version_from_electron_builder_yaml).to be_a(String) + it "returns an array of version strings when given YAML text" do + expect(electron_builder.versions_from_content(electron_builder_yaml)).to eq(versions) end - it "returns a version string when given YAML data and a block" do - version = electron_builder.version_from_content(electron_builder_yaml) do |yaml| - yaml["version"].sub("3", "4") - end + it "returns an array of version strings when given YAML text and a block" do + # Returning a string from block + expect( + electron_builder.versions_from_content(electron_builder_yaml) do |yaml| + yaml["version"].sub("3", "4") + end, + ).to eq(["1.2.4"]) - expect(version).to eq "1.2.4" + # Returning an array of strings from block + expect(electron_builder.versions_from_content(electron_builder_yaml) { versions }).to eq(versions) end - it "allows a nil return from a strategy block" do - expect(electron_builder.version_from_content(electron_builder_yaml) { next }).to eq(nil) + it "allows a nil return from a block" do + expect(electron_builder.versions_from_content(electron_builder_yaml) { next }).to eq([]) end - it "errors on an invalid return type from a strategy block" do - expect { electron_builder.version_from_content(electron_builder_yaml) { 123 } } - .to raise_error(TypeError, "Return value of `strategy :electron_builder` block must be a string.") + it "errors on an invalid return type from a block" do + expect { electron_builder.versions_from_content(electron_builder_yaml) { 123 } } + .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG) end end end diff --git a/Library/Homebrew/test/livecheck/strategy/extract_plist_spec.rb b/Library/Homebrew/test/livecheck/strategy/extract_plist_spec.rb new file mode 100644 index 0000000000..383f45b47a --- /dev/null +++ b/Library/Homebrew/test/livecheck/strategy/extract_plist_spec.rb @@ -0,0 +1,72 @@ +# typed: false +# frozen_string_literal: true + +require "livecheck/strategy" +require "bundle_version" + +describe Homebrew::Livecheck::Strategy::ExtractPlist do + subject(:extract_plist) { described_class } + + let(:http_url) { "https://brew.sh/blog/" } + let(:non_http_url) { "ftp://brew.sh/" } + + let(:items) do + { + "first" => extract_plist::Item.new( + bundle_version: Homebrew::BundleVersion.new(nil, "1.2"), + ), + "second" => extract_plist::Item.new( + bundle_version: Homebrew::BundleVersion.new(nil, "1.2.3"), + ), + } + end + + let(:versions) { ["1.2", "1.2.3"] } + + describe "::match?" do + it "returns true for an HTTP URL" do + expect(extract_plist.match?(http_url)).to be true + end + + it "returns false for a non-HTTP URL" do + expect(extract_plist.match?(non_http_url)).to be false + end + end + + describe "::versions_from_items" do + it "returns an empty array if Items hash is empty" do + expect(extract_plist.versions_from_items({})).to eq([]) + end + + it "returns an array of version strings when given Items" do + expect(extract_plist.versions_from_items(items)).to eq(versions) + end + + it "returns an array of version strings when given Items and a block" do + # Returning a string from block + expect( + extract_plist.versions_from_items(items) do |items| + items["first"].version + end, + ).to eq(["1.2"]) + + # Returning an array of strings from block + expect( + extract_plist.versions_from_items(items) do |items| + items.map do |_key, item| + item.bundle_version.nice_version + end + end, + ).to eq(versions) + end + + it "allows a nil return from a block" do + expect(extract_plist.versions_from_items(items) { next }).to eq([]) + end + + it "errors on an invalid return type from a block" do + expect { extract_plist.versions_from_items(items) { 123 } } + .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG) + end + end +end diff --git a/Library/Homebrew/test/livecheck/strategy/git_spec.rb b/Library/Homebrew/test/livecheck/strategy/git_spec.rb index 9a0996a2d8..2a5f9e03b1 100644 --- a/Library/Homebrew/test/livecheck/strategy/git_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/git_spec.rb @@ -1,7 +1,7 @@ # typed: false # frozen_string_literal: true -require "livecheck/strategy/git" +require "livecheck/strategy" describe Homebrew::Livecheck::Strategy::Git do subject(:git) { described_class } @@ -9,10 +9,32 @@ describe Homebrew::Livecheck::Strategy::Git do let(:git_url) { "https://github.com/Homebrew/brew.git" } let(:non_git_url) { "https://brew.sh/test" } + let(:tags) { + { + normal: ["brew/1.2", "brew/1.2.1", "brew/1.2.2", "brew/1.2.3", "brew/1.2.4", "1.2.5"], + hyphens: ["brew/1-2", "brew/1-2-1", "brew/1-2-2", "brew/1-2-3", "brew/1-2-4", "1-2-5"], + } + } + + let(:regexes) { + { + standard: /^v?(\d+(?:\.\d+)+)$/i, + hyphens: /^v?(\d+(?:[.-]\d+)+)$/i, + brew: %r{^brew/v?(\d+(?:\.\d+)+)$}i, + } + } + + let(:versions) { + { + default: ["1.2", "1.2.1", "1.2.2", "1.2.3", "1.2.4", "1.2.5"], + standard_regex: ["1.2.5"], + brew_regex: ["1.2", "1.2.1", "1.2.2", "1.2.3", "1.2.4"], + } + } + describe "::tag_info", :needs_network do it "returns the Git tags for the provided remote URL that match the regex provided" do - expect(git.tag_info(git_url, /^v?(\d+(?:\.\d+))$/)) - .not_to be_empty + expect(git.tag_info(git_url, regexes[:standard])).not_to be_empty end end @@ -25,4 +47,46 @@ describe Homebrew::Livecheck::Strategy::Git do expect(git.match?(non_git_url)).to be false end end + + describe "::versions_from_tags" do + it "returns an empty array if tags array is empty" do + expect(git.versions_from_tags([])).to eq([]) + end + + it "returns an array of version strings when given tags" do + expect(git.versions_from_tags(tags[:normal])).to eq(versions[:default]) + expect(git.versions_from_tags(tags[:normal], regexes[:standard])).to eq(versions[:standard_regex]) + expect(git.versions_from_tags(tags[:normal], regexes[:brew])).to eq(versions[:brew_regex]) + end + + it "returns an array of version strings when given tags and a block" do + # Returning a string from block, default strategy regex + expect(git.versions_from_tags(tags[:normal]) { versions[:default].first }).to eq([versions[:default].first]) + + # Returning an array of strings from block, default strategy regex + expect( + git.versions_from_tags(tags[:hyphens]) do |tags, regex| + tags.map { |tag| tag[regex, 1]&.tr("-", ".") } + end, + ).to eq(versions[:default]) + + # Returning an array of strings from block, explicit regex + expect( + git.versions_from_tags(tags[:hyphens], regexes[:hyphens]) do |tags, regex| + tags.map { |tag| tag[regex, 1]&.tr("-", ".") } + end, + ).to eq(versions[:standard_regex]) + + expect(git.versions_from_tags(tags[:hyphens]) { "1.2.3" }).to eq(["1.2.3"]) + end + + it "allows a nil return from a block" do + expect(git.versions_from_tags(tags[:normal]) { next }).to eq([]) + end + + it "errors on an invalid return type from a block" do + expect { git.versions_from_tags(tags[:normal]) { 123 } } + .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG) + end + end end diff --git a/Library/Homebrew/test/livecheck/strategy/header_match_spec.rb b/Library/Homebrew/test/livecheck/strategy/header_match_spec.rb index fbb38b8366..65b34c5ff4 100644 --- a/Library/Homebrew/test/livecheck/strategy/header_match_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/header_match_spec.rb @@ -1,16 +1,111 @@ # typed: false # frozen_string_literal: true -require "livecheck/strategy/header_match" +require "livecheck/strategy" describe Homebrew::Livecheck::Strategy::HeaderMatch do subject(:header_match) { described_class } let(:url) { "https://www.example.com/" } + let(:versions) { + versions = { + content_disposition: ["1.2.3"], + location: ["1.2.4"], + } + versions[:content_disposition_and_location] = versions[:content_disposition] + versions[:location] + + versions + } + + let(:headers) { + headers = { + content_disposition: { + "date" => "Fri, 01 Jan 2021 01:23:45 GMT", + "content-type" => "application/x-gzip", + "content-length" => "120", + "content-disposition" => "attachment; filename=brew-#{versions[:content_disposition].first}.tar.gz", + }, + location: { + "date" => "Fri, 01 Jan 2021 01:23:45 GMT", + "content-type" => "text/html; charset=utf-8", + "location" => "https://github.com/Homebrew/brew/releases/tag/#{versions[:location].first}", + "content-length" => "117", + }, + } + headers[:content_disposition_and_location] = headers[:content_disposition].merge(headers[:location]) + + headers + } + + let(:regexes) { + { + archive: /filename=brew[._-]v?(\d+(?:\.\d+)+)\.t/i, + latest: %r{.*?/tag/v?(\d+(?:\.\d+)+)$}i, + loose: /v?(\d+(?:\.\d+)+)/i, + } + } + describe "::match?" do it "returns true for any URL" do expect(header_match.match?(url)).to be true end end + + describe "::versions_from_headers" do + it "returns an empty array if headers hash is empty" do + expect(header_match.versions_from_headers({})).to eq([]) + end + + it "returns an array of version strings when given headers" do + expect(header_match.versions_from_headers(headers[:content_disposition])).to eq(versions[:content_disposition]) + expect(header_match.versions_from_headers(headers[:location])).to eq(versions[:location]) + expect(header_match.versions_from_headers(headers[:content_disposition_and_location])) + .to eq(versions[:content_disposition_and_location]) + + expect(header_match.versions_from_headers(headers[:content_disposition], regexes[:archive])) + .to eq(versions[:content_disposition]) + expect(header_match.versions_from_headers(headers[:location], regexes[:latest])).to eq(versions[:location]) + expect(header_match.versions_from_headers(headers[:content_disposition_and_location], regexes[:latest])) + .to eq(versions[:location]) + end + + it "returns an array of version strings when given headers and a block" do + # Returning a string from block, no regex + expect( + header_match.versions_from_headers(headers[:location]) do |headers| + v = Version.parse(headers["location"], detected_from_url: true) + v.null? ? nil : v.to_s + end, + ).to eq(versions[:location]) + + # Returning a string from block, explicit regex + expect( + header_match.versions_from_headers(headers[:location], regexes[:latest]) do |headers, regex| + headers["location"] ? headers["location"][regex, 1] : nil + end, + ).to eq(versions[:location]) + + # Returning an array of strings from block + # NOTE: Strategies runs `#compact` on an array from a block, so nil + # values are filtered out without needing to use `#compact` in the block. + expect( + header_match.versions_from_headers( + headers[:content_disposition_and_location], + regexes[:loose], + ) do |headers, regex| + headers.transform_values { |header| header[regex, 1] }.values + end, + ).to eq(versions[:content_disposition_and_location]) + end + + it "allows a nil return from a block" do + expect(header_match.versions_from_headers(headers[:location]) { next }).to eq([]) + end + + it "errors on an invalid return type from a block" do + expect { header_match.versions_from_headers(headers) { 123 } } + .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG) + end + end end diff --git a/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb b/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb index 24e5705d0e..b9222ff2fc 100644 --- a/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb @@ -1,7 +1,7 @@ # typed: false # frozen_string_literal: true -require "livecheck/strategy/page_match" +require "livecheck/strategy" describe Homebrew::Livecheck::Strategy::PageMatch do subject(:page_match) { described_class } @@ -9,7 +9,7 @@ describe Homebrew::Livecheck::Strategy::PageMatch do let(:url) { "https://brew.sh/blog/" } let(:regex) { %r{href=.*?/homebrew[._-]v?(\d+(?:\.\d+)+)/?["' >]}i } - let(:page_content) { + let(:content) { <<~EOS @@ -35,7 +35,7 @@ describe Homebrew::Livecheck::Strategy::PageMatch do EOS } - let(:page_content_matches) { ["2.6.0", "2.5.0", "2.4.0", "2.3.0", "2.2.0", "2.1.0", "2.0.0", "1.9.0"] } + let(:content_matches) { ["2.6.0", "2.5.0", "2.4.0", "2.3.0", "2.2.0", "2.1.0", "2.0.0", "1.9.0"] } let(:find_versions_return_hash) { { @@ -66,24 +66,41 @@ describe Homebrew::Livecheck::Strategy::PageMatch do end end - describe "::page_matches" do - it "finds matching text in page content using a regex" do - expect(page_match.page_matches(page_content, regex)).to eq(page_content_matches) + describe "::versions_from_content" do + it "returns an empty array if content is blank" do + expect(page_match.versions_from_content("", regex)).to eq([]) end - it "finds matching text in page content using a strategy block" do - expect(page_match.page_matches(page_content, regex) { |content, regex| content.scan(regex).map(&:first).uniq }) - .to eq(page_content_matches) + it "returns an array of version strings when given content" do + expect(page_match.versions_from_content(content, regex)).to eq(content_matches) + + # Regexes should use a capture group around the version but a regex + # without one should still be handled + expect(page_match.versions_from_content(content, /\d+(?:\.\d+)+/i)).to eq(content_matches) end - it "allows a nil return from a strategy block" do - expect(page_match.page_matches(page_content, regex) { next }).to eq([]) + it "returns an array of version strings when given content and a block" do + # Returning a string from block + expect(page_match.versions_from_content(content, regex) { "1.2.3" }).to eq(["1.2.3"]) + + # Returning an array of strings from block + expect(page_match.versions_from_content(content, regex) { |page, regex| page.scan(regex).map(&:first) }) + .to eq(content_matches) + end + + it "allows a nil return from a block" do + expect(page_match.versions_from_content(content, regex) { next }).to eq([]) + end + + it "errors on an invalid return type from a block" do + expect { page_match.versions_from_content(content, regex) { 123 } } + .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG) end end describe "::find_versions?" do it "finds versions in provided_content" do - expect(page_match.find_versions(url, regex, provided_content: page_content)) + expect(page_match.find_versions(url, regex, provided_content: content)) .to eq(find_versions_cached_return_hash) end end diff --git a/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb b/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb index 47d7f00df5..0931c98ab2 100644 --- a/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb @@ -1,7 +1,8 @@ # typed: false # frozen_string_literal: true -require "livecheck/strategy/sparkle" +require "livecheck/strategy" +require "bundle_version" describe Homebrew::Livecheck::Strategy::Sparkle do subject(:sparkle) { described_class } @@ -11,6 +12,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do let(:appcast_data) { { title: "Version 1.2.3", + pub_date: "Fri, 01 Jan 2021 01:23:45 +0000", url: "https://www.example.com/example/example.tar.gz", short_version: "1.2.3", version: "1234", @@ -30,6 +32,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do #{appcast_data[:title]} 10.10 https://www.example.com/example/1.2.3.html + #{appcast_data[:pub_date]} @@ -37,6 +40,17 @@ describe Homebrew::Livecheck::Strategy::Sparkle do EOS } + let(:item) { + Homebrew::Livecheck::Strategy::Sparkle::Item.new( + title: appcast_data[:title], + pub_date: Time.parse(appcast_data[:pub_date]), + url: appcast_data[:url], + bundle_version: Homebrew::BundleVersion.new(appcast_data[:short_version], appcast_data[:version]), + ) + } + + let(:versions) { [item.bundle_version.nice_version] } + describe "::match?" do it "returns true for any URL" do expect(sparkle.match?(url)).to be true @@ -52,10 +66,39 @@ describe Homebrew::Livecheck::Strategy::Sparkle do it "returns an Item when given XML data" do expect(item_from_appcast_xml).to be_a(Homebrew::Livecheck::Strategy::Sparkle::Item) + expect(item_from_appcast_xml).to eq(item) expect(item_from_appcast_xml.title).to eq(appcast_data[:title]) + expect(item_from_appcast_xml.pub_date).to eq(Time.parse(appcast_data[:pub_date])) expect(item_from_appcast_xml.url).to eq(appcast_data[:url]) expect(item_from_appcast_xml.short_version).to eq(appcast_data[:short_version]) expect(item_from_appcast_xml.version).to eq(appcast_data[:version]) end end + + describe "::versions_from_content" do + it "returns an array of version strings when given content" do + expect(sparkle.versions_from_content(appcast_xml)).to eq(versions) + end + + it "returns an array of version strings when given content and a block" do + # Returning a string from block + expect( + sparkle.versions_from_content(appcast_xml) do |item| + item.bundle_version&.nice_version&.sub("3", "4") + end, + ).to eq([item.bundle_version.nice_version.sub("3", "4")]) + + # Returning an array of strings from block + expect(sparkle.versions_from_content(appcast_xml) { versions }).to eq(versions) + end + + it "allows a nil return from a block" do + expect(sparkle.versions_from_content(appcast_xml) { next }).to eq([]) + end + + it "errors on an invalid return type from a block" do + expect { sparkle.versions_from_content(appcast_xml) { 123 } } + .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG) + end + end end diff --git a/Library/Homebrew/test/livecheck/strategy_spec.rb b/Library/Homebrew/test/livecheck/strategy_spec.rb index a2480b3403..7dd926fb40 100644 --- a/Library/Homebrew/test/livecheck/strategy_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy_spec.rb @@ -30,4 +30,20 @@ describe Homebrew::Livecheck::Strategy do end end end + + describe "::handle_block_return" do + it "returns an array of version strings when given a valid value" do + expect(strategy.handle_block_return("1.2.3")).to eq(["1.2.3"]) + expect(strategy.handle_block_return(["1.2.3", "1.2.4"])).to eq(["1.2.3", "1.2.4"]) + end + + it "returns an empty array when given a nil value" do + expect(strategy.handle_block_return(nil)).to eq([]) + end + + it "errors when given an invalid value" do + expect { strategy.handle_block_return(123) } + .to raise_error(TypeError, strategy::INVALID_BLOCK_RETURN_VALUE_MSG) + end + end end From 7e07010f06a594f72cbf12a5cae889eaa15c4373 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Tue, 10 Aug 2021 17:39:11 -0400 Subject: [PATCH 3/6] Improve standardization of strategy tests --- .../test/livecheck/strategy/apache_spec.rb | 4 ++-- .../test/livecheck/strategy/bitbucket_spec.rb | 4 ++-- .../test/livecheck/strategy/cpan_spec.rb | 4 ++-- .../livecheck/strategy/electron_builder_spec.rb | 12 ++++++------ .../Homebrew/test/livecheck/strategy/git_spec.rb | 4 ++-- .../livecheck/strategy/github_latest_spec.rb | 8 ++++---- .../test/livecheck/strategy/gnome_spec.rb | 4 ++-- .../Homebrew/test/livecheck/strategy/gnu_spec.rb | 6 +++--- .../test/livecheck/strategy/hackage_spec.rb | 4 ++-- .../test/livecheck/strategy/header_match_spec.rb | 11 ++++++++--- .../test/livecheck/strategy/launchpad_spec.rb | 4 ++-- .../Homebrew/test/livecheck/strategy/npm_spec.rb | 4 ++-- .../test/livecheck/strategy/page_match_spec.rb | 16 +++++++++++----- .../test/livecheck/strategy/pypi_spec.rb | 4 ++-- .../test/livecheck/strategy/sourceforge_spec.rb | 4 ++-- .../test/livecheck/strategy/sparkle_spec.rb | 13 +++++++++---- .../test/livecheck/strategy/xorg_spec.rb | 4 ++-- 17 files changed, 63 insertions(+), 47 deletions(-) diff --git a/Library/Homebrew/test/livecheck/strategy/apache_spec.rb b/Library/Homebrew/test/livecheck/strategy/apache_spec.rb index 7bd4d9ae1e..f7c3ebc7d5 100644 --- a/Library/Homebrew/test/livecheck/strategy/apache_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/apache_spec.rb @@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Apache do let(:non_apache_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is an Apache URL" do + it "returns true for an Apache URL" do expect(apache.match?(apache_url)).to be true end - it "returns false if the argument provided is not an Apache URL" do + it "returns false for a non-Apache URL" do expect(apache.match?(non_apache_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/bitbucket_spec.rb b/Library/Homebrew/test/livecheck/strategy/bitbucket_spec.rb index f5c2df857f..341f8240f1 100644 --- a/Library/Homebrew/test/livecheck/strategy/bitbucket_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/bitbucket_spec.rb @@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Bitbucket do let(:non_bitbucket_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is a Bitbucket URL" do + it "returns true for a Bitbucket URL" do expect(bitbucket.match?(bitbucket_url)).to be true end - it "returns false if the argument provided is not a Bitbucket URL" do + it "returns false for a non-Bitbucket URL" do expect(bitbucket.match?(non_bitbucket_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/cpan_spec.rb b/Library/Homebrew/test/livecheck/strategy/cpan_spec.rb index b119216c86..29af780d20 100644 --- a/Library/Homebrew/test/livecheck/strategy/cpan_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/cpan_spec.rb @@ -11,12 +11,12 @@ describe Homebrew::Livecheck::Strategy::Cpan do let(:non_cpan_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is a CPAN URL" do + it "returns true for a CPAN URL" do expect(cpan.match?(cpan_url_no_subdirectory)).to be true expect(cpan.match?(cpan_url_with_subdirectory)).to be true end - it "returns false if the argument provided is not a CPAN URL" do + it "returns false for a non-CPAN URL" do expect(cpan.match?(non_cpan_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb b/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb index d76f53d670..801636563b 100644 --- a/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb @@ -6,8 +6,8 @@ require "livecheck/strategy" describe Homebrew::Livecheck::Strategy::ElectronBuilder do subject(:electron_builder) { described_class } - let(:valid_url) { "https://www.example.com/example/latest-mac.yml" } - let(:invalid_url) { "https://brew.sh/test" } + let(:yaml_url) { "https://www.example.com/example/latest-mac.yml" } + let(:non_yaml_url) { "https://brew.sh/test" } let(:electron_builder_yaml) { <<~EOS @@ -29,12 +29,12 @@ describe Homebrew::Livecheck::Strategy::ElectronBuilder do let(:versions) { ["1.2.3"] } describe "::match?" do - it "returns true for any URL pointing to a YAML file" do - expect(electron_builder.match?(valid_url)).to be true + it "returns true for a YAML file URL" do + expect(electron_builder.match?(yaml_url)).to be true end - it "returns false for a URL not pointing to a YAML file" do - expect(electron_builder.match?(invalid_url)).to be false + it "returns false for non-YAML URL" do + expect(electron_builder.match?(non_yaml_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/git_spec.rb b/Library/Homebrew/test/livecheck/strategy/git_spec.rb index 2a5f9e03b1..cb1a0106a1 100644 --- a/Library/Homebrew/test/livecheck/strategy/git_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/git_spec.rb @@ -39,11 +39,11 @@ describe Homebrew::Livecheck::Strategy::Git do end describe "::match?" do - it "returns true if the argument provided is a Git repository" do + it "returns true for a Git repository URL" do expect(git.match?(git_url)).to be true end - it "returns false if the argument provided is not a Git repository" do + it "returns false for a non-Git URL" do expect(git.match?(non_git_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/github_latest_spec.rb b/Library/Homebrew/test/livecheck/strategy/github_latest_spec.rb index d15088049a..e744b04839 100644 --- a/Library/Homebrew/test/livecheck/strategy/github_latest_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/github_latest_spec.rb @@ -14,19 +14,19 @@ describe Homebrew::Livecheck::Strategy::GithubLatest do let(:non_github_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is a GitHub release artifact URL" do + it "returns true for a GitHub release artifact URL" do expect(github_latest.match?(github_release_artifact_url)).to be true end - it "returns true if the argument provided is a GitHub tag archive URL" do + it "returns true for a GitHub tag archive URL" do expect(github_latest.match?(github_tag_archive_url)).to be true end - it "returns true if the argument provided is a GitHub repository upload URL" do + it "returns true for a GitHub repository upload URL" do expect(github_latest.match?(github_repository_upload_url)).to be true end - it "returns false if the argument provided is not a GitHub URL" do + it "returns false for a non-GitHub URL" do expect(github_latest.match?(non_github_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/gnome_spec.rb b/Library/Homebrew/test/livecheck/strategy/gnome_spec.rb index 4edf101613..394a5da473 100644 --- a/Library/Homebrew/test/livecheck/strategy/gnome_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/gnome_spec.rb @@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Gnome do let(:non_gnome_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is a GNOME URL" do + it "returns true for a GNOME URL" do expect(gnome.match?(gnome_url)).to be true end - it "returns false if the argument provided is not a GNOME URL" do + it "returns false for a non-GNOME URL" do expect(gnome.match?(non_gnome_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/gnu_spec.rb b/Library/Homebrew/test/livecheck/strategy/gnu_spec.rb index 63e28de011..a235eade20 100644 --- a/Library/Homebrew/test/livecheck/strategy/gnu_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/gnu_spec.rb @@ -11,15 +11,15 @@ describe Homebrew::Livecheck::Strategy::Gnu do let(:non_gnu_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is a non-Savannah GNU URL" do + it "returns true for a [non-Savannah] GNU URL" do expect(gnu.match?(gnu_url)).to be true end - it "returns false if the argument provided is a Savannah GNU URL" do + it "returns false for a Savannah GNU URL" do expect(gnu.match?(savannah_gnu_url)).to be false end - it "returns false if the argument provided is not a GNU URL" do + it "returns false for a non-GNU URL (not nongnu.org)" do expect(gnu.match?(non_gnu_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/hackage_spec.rb b/Library/Homebrew/test/livecheck/strategy/hackage_spec.rb index 7a393294a7..f9eb06a35b 100644 --- a/Library/Homebrew/test/livecheck/strategy/hackage_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/hackage_spec.rb @@ -11,12 +11,12 @@ describe Homebrew::Livecheck::Strategy::Hackage do let(:non_hackage_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is a Hackage URL" do + it "returns true for a Hackage URL" do expect(hackage.match?(hackage_url)).to be true expect(hackage.match?(hackage_downloads_url)).to be true end - it "returns false if the argument provided is not a Hackage URL" do + it "returns false for a non-Hackage URL" do expect(hackage.match?(non_hackage_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/header_match_spec.rb b/Library/Homebrew/test/livecheck/strategy/header_match_spec.rb index 65b34c5ff4..7ce7ccf37d 100644 --- a/Library/Homebrew/test/livecheck/strategy/header_match_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/header_match_spec.rb @@ -6,7 +6,8 @@ require "livecheck/strategy" describe Homebrew::Livecheck::Strategy::HeaderMatch do subject(:header_match) { described_class } - let(:url) { "https://www.example.com/" } + let(:http_url) { "https://brew.sh/blog/" } + let(:non_http_url) { "ftp://brew.sh/" } let(:versions) { versions = { @@ -47,8 +48,12 @@ describe Homebrew::Livecheck::Strategy::HeaderMatch do } describe "::match?" do - it "returns true for any URL" do - expect(header_match.match?(url)).to be true + it "returns true for an HTTP URL" do + expect(header_match.match?(http_url)).to be true + end + + it "returns false for a non-HTTP URL" do + expect(header_match.match?(non_http_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/launchpad_spec.rb b/Library/Homebrew/test/livecheck/strategy/launchpad_spec.rb index 150c547784..c55a2144b6 100644 --- a/Library/Homebrew/test/livecheck/strategy/launchpad_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/launchpad_spec.rb @@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Launchpad do let(:non_launchpad_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is a Launchpad URL" do + it "returns true for a Launchpad URL" do expect(launchpad.match?(launchpad_url)).to be true end - it "returns false if the argument provided is not a Launchpad URL" do + it "returns false for a non-Launchpad URL" do expect(launchpad.match?(non_launchpad_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/npm_spec.rb b/Library/Homebrew/test/livecheck/strategy/npm_spec.rb index 2ed3280633..d07f28367d 100644 --- a/Library/Homebrew/test/livecheck/strategy/npm_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/npm_spec.rb @@ -11,12 +11,12 @@ describe Homebrew::Livecheck::Strategy::Npm do let(:non_npm_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is an npm URL" do + it "returns true for an npm URL" do expect(npm.match?(npm_url)).to be true expect(npm.match?(npm_scoped_url)).to be true end - it "returns false if the argument provided is not an npm URL" do + it "returns false for a non-npm URL" do expect(npm.match?(non_npm_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb b/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb index b9222ff2fc..b7225ba3de 100644 --- a/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb @@ -6,7 +6,9 @@ require "livecheck/strategy" describe Homebrew::Livecheck::Strategy::PageMatch do subject(:page_match) { described_class } - let(:url) { "https://brew.sh/blog/" } + let(:http_url) { "https://brew.sh/blog/" } + let(:non_http_url) { "ftp://brew.sh/" } + let(:regex) { %r{href=.*?/homebrew[._-]v?(\d+(?:\.\d+)+)/?["' >]}i } let(:content) { @@ -50,7 +52,7 @@ describe Homebrew::Livecheck::Strategy::PageMatch do "1.9.0" => Version.new("1.9.0"), }, regex: regex, - url: url, + url: http_url, } } @@ -61,8 +63,12 @@ describe Homebrew::Livecheck::Strategy::PageMatch do } describe "::match?" do - it "returns true for any URL" do - expect(page_match.match?(url)).to be true + it "returns true for an HTTP URL" do + expect(page_match.match?(http_url)).to be true + end + + it "returns false for a non-HTTP URL" do + expect(page_match.match?(non_http_url)).to be false end end @@ -100,7 +106,7 @@ describe Homebrew::Livecheck::Strategy::PageMatch do describe "::find_versions?" do it "finds versions in provided_content" do - expect(page_match.find_versions(url, regex, provided_content: content)) + expect(page_match.find_versions(http_url, regex, provided_content: content)) .to eq(find_versions_cached_return_hash) end end diff --git a/Library/Homebrew/test/livecheck/strategy/pypi_spec.rb b/Library/Homebrew/test/livecheck/strategy/pypi_spec.rb index e8a4ba1b87..9f373829de 100644 --- a/Library/Homebrew/test/livecheck/strategy/pypi_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/pypi_spec.rb @@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Pypi do let(:non_pypi_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is a PyPI URL" do + it "returns true for a PyPI URL" do expect(pypi.match?(pypi_url)).to be true end - it "returns false if the argument provided is not a PyPI URL" do + it "returns false for a non-PyPI URL" do expect(pypi.match?(non_pypi_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/sourceforge_spec.rb b/Library/Homebrew/test/livecheck/strategy/sourceforge_spec.rb index c37c9cdc2e..94e4c0167b 100644 --- a/Library/Homebrew/test/livecheck/strategy/sourceforge_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/sourceforge_spec.rb @@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Sourceforge do let(:non_sourceforge_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is a SourceForge URL" do + it "returns true for a SourceForge URL" do expect(sourceforge.match?(sourceforge_url)).to be true end - it "returns false if the argument provided is not a SourceForge URL" do + it "returns false for a non-SourceForge URL" do expect(sourceforge.match?(non_sourceforge_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb b/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb index 0931c98ab2..1a572c827d 100644 --- a/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb @@ -7,7 +7,8 @@ require "bundle_version" describe Homebrew::Livecheck::Strategy::Sparkle do subject(:sparkle) { described_class } - let(:url) { "https://www.example.com/example/appcast.xml" } + let(:appcast_url) { "https://www.example.com/example/appcast.xml" } + let(:non_http_url) { "ftp://brew.sh/" } let(:appcast_data) { { @@ -25,7 +26,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do Example Changelog - #{url} + #{appcast_url} Most recent changes with links to updates. en @@ -52,8 +53,12 @@ describe Homebrew::Livecheck::Strategy::Sparkle do let(:versions) { [item.bundle_version.nice_version] } describe "::match?" do - it "returns true for any URL" do - expect(sparkle.match?(url)).to be true + it "returns true for an HTTP URL" do + expect(sparkle.match?(appcast_url)).to be true + end + + it "returns false for a non-HTTP URL" do + expect(sparkle.match?(non_http_url)).to be false end end diff --git a/Library/Homebrew/test/livecheck/strategy/xorg_spec.rb b/Library/Homebrew/test/livecheck/strategy/xorg_spec.rb index 664797732b..2eee435f49 100644 --- a/Library/Homebrew/test/livecheck/strategy/xorg_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/xorg_spec.rb @@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Xorg do let(:non_xorg_url) { "https://brew.sh/test" } describe "::match?" do - it "returns true if the argument provided is an X.Org URL" do + it "returns true for an X.Org URL" do expect(xorg.match?(xorg_url)).to be true end - it "returns false if the argument provided is not an X.Org URL" do + it "returns false for a non-X.Org URL" do expect(xorg.match?(non_xorg_url)).to be false end end From f2bd39ccefd89a76c20c32b00fc6f3455885b6e0 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Tue, 10 Aug 2021 18:24:51 -0400 Subject: [PATCH 4/6] Add additional method signatures to strategies --- Library/Homebrew/livecheck/strategy.rb | 16 ++++++++++++++-- Library/Homebrew/livecheck/strategy/apache.rb | 1 + Library/Homebrew/livecheck/strategy/bitbucket.rb | 1 + Library/Homebrew/livecheck/strategy/cpan.rb | 1 + Library/Homebrew/livecheck/strategy/git.rb | 1 + .../Homebrew/livecheck/strategy/github_latest.rb | 1 + Library/Homebrew/livecheck/strategy/gnome.rb | 1 + Library/Homebrew/livecheck/strategy/gnu.rb | 1 + Library/Homebrew/livecheck/strategy/hackage.rb | 1 + Library/Homebrew/livecheck/strategy/launchpad.rb | 1 + Library/Homebrew/livecheck/strategy/npm.rb | 1 + Library/Homebrew/livecheck/strategy/pypi.rb | 1 + .../Homebrew/livecheck/strategy/sourceforge.rb | 1 + Library/Homebrew/livecheck/strategy/xorg.rb | 1 + 14 files changed, 27 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/livecheck/strategy.rb b/Library/Homebrew/livecheck/strategy.rb index 0040bc4821..b77610d0c4 100644 --- a/Library/Homebrew/livecheck/strategy.rb +++ b/Library/Homebrew/livecheck/strategy.rb @@ -86,6 +86,7 @@ module Homebrew # At present, this should only be called after tap strategies have been # loaded, otherwise livecheck won't be able to use them. # @return [Hash] + sig { returns(T::Hash[Symbol, T.untyped]) } def strategies return @strategies if defined? @strategies @@ -107,8 +108,9 @@ module Homebrew # @param symbol [Symbol] the strategy name in snake case as a `Symbol` # (e.g. `:page_match`) # @return [Strategy, nil] + sig { params(symbol: T.nilable(Symbol)).returns(T.nilable(T.untyped)) } def from_symbol(symbol) - strategies[symbol] + strategies[symbol] if symbol.present? end # Returns an array of strategies that apply to the provided URL. @@ -119,7 +121,16 @@ module Homebrew # @param regex_provided [Boolean] whether a regex is provided in the # `livecheck` block # @return [Array] - def from_url(url, livecheck_strategy: nil, url_provided: nil, regex_provided: nil, block_provided: nil) + sig { + params( + url: String, + livecheck_strategy: T.nilable(Symbol), + url_provided: T::Boolean, + regex_provided: T::Boolean, + block_provided: T::Boolean, + ).returns(T::Array[T.untyped]) + } + def from_url(url, livecheck_strategy: nil, url_provided: false, regex_provided: false, block_provided: false) usable_strategies = strategies.values.select do |strategy| if strategy == PageMatch # Only treat the `PageMatch` strategy as usable if a regex is @@ -143,6 +154,7 @@ module Homebrew end end + sig { params(url: String).returns(T::Array[T::Hash[String, String]]) } def self.page_headers(url) headers = [] diff --git a/Library/Homebrew/livecheck/strategy/apache.rb b/Library/Homebrew/livecheck/strategy/apache.rb index 36b2395094..6cd4e29416 100644 --- a/Library/Homebrew/livecheck/strategy/apache.rb +++ b/Library/Homebrew/livecheck/strategy/apache.rb @@ -37,6 +37,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/bitbucket.rb b/Library/Homebrew/livecheck/strategy/bitbucket.rb index 4072748aed..a3fc074af2 100644 --- a/Library/Homebrew/livecheck/strategy/bitbucket.rb +++ b/Library/Homebrew/livecheck/strategy/bitbucket.rb @@ -44,6 +44,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/cpan.rb b/Library/Homebrew/livecheck/strategy/cpan.rb index 88965bd8d6..f1fd982822 100644 --- a/Library/Homebrew/livecheck/strategy/cpan.rb +++ b/Library/Homebrew/livecheck/strategy/cpan.rb @@ -35,6 +35,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/git.rb b/Library/Homebrew/livecheck/strategy/git.rb index 55c3c05562..d68853ab9b 100644 --- a/Library/Homebrew/livecheck/strategy/git.rb +++ b/Library/Homebrew/livecheck/strategy/git.rb @@ -50,6 +50,7 @@ module Homebrew # @param url [String] the URL of the Git repository to check # @param regex [Regexp] the regex to use for filtering tags # @return [Hash] + sig { params(url: String, regex: T.nilable(Regexp)).returns(T::Hash[Symbol, T.untyped]) } def self.tag_info(url, regex = nil) # Open3#capture3 is used here because we need to capture stderr # output and handle it in an appropriate manner. Alternatives like diff --git a/Library/Homebrew/livecheck/strategy/github_latest.rb b/Library/Homebrew/livecheck/strategy/github_latest.rb index 68a9657448..c575bf55bb 100644 --- a/Library/Homebrew/livecheck/strategy/github_latest.rb +++ b/Library/Homebrew/livecheck/strategy/github_latest.rb @@ -52,6 +52,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/gnome.rb b/Library/Homebrew/livecheck/strategy/gnome.rb index 7af002b9f5..b8bd9621cb 100644 --- a/Library/Homebrew/livecheck/strategy/gnome.rb +++ b/Library/Homebrew/livecheck/strategy/gnome.rb @@ -40,6 +40,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/gnu.rb b/Library/Homebrew/livecheck/strategy/gnu.rb index dac3db3ef6..71c778bc0a 100644 --- a/Library/Homebrew/livecheck/strategy/gnu.rb +++ b/Library/Homebrew/livecheck/strategy/gnu.rb @@ -44,6 +44,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) && url.exclude?("savannah.") end diff --git a/Library/Homebrew/livecheck/strategy/hackage.rb b/Library/Homebrew/livecheck/strategy/hackage.rb index 025ebe266a..fe99e66fe5 100644 --- a/Library/Homebrew/livecheck/strategy/hackage.rb +++ b/Library/Homebrew/livecheck/strategy/hackage.rb @@ -37,6 +37,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/launchpad.rb b/Library/Homebrew/livecheck/strategy/launchpad.rb index d4f935b90e..ee91769c8c 100644 --- a/Library/Homebrew/livecheck/strategy/launchpad.rb +++ b/Library/Homebrew/livecheck/strategy/launchpad.rb @@ -35,6 +35,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/npm.rb b/Library/Homebrew/livecheck/strategy/npm.rb index db3b062247..4faab03ca2 100644 --- a/Library/Homebrew/livecheck/strategy/npm.rb +++ b/Library/Homebrew/livecheck/strategy/npm.rb @@ -31,6 +31,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/pypi.rb b/Library/Homebrew/livecheck/strategy/pypi.rb index ecbd14c505..96a5b1fbf7 100644 --- a/Library/Homebrew/livecheck/strategy/pypi.rb +++ b/Library/Homebrew/livecheck/strategy/pypi.rb @@ -41,6 +41,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/sourceforge.rb b/Library/Homebrew/livecheck/strategy/sourceforge.rb index a6a9a9add2..0ffcdb0d2e 100644 --- a/Library/Homebrew/livecheck/strategy/sourceforge.rb +++ b/Library/Homebrew/livecheck/strategy/sourceforge.rb @@ -47,6 +47,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end diff --git a/Library/Homebrew/livecheck/strategy/xorg.rb b/Library/Homebrew/livecheck/strategy/xorg.rb index 6f6b94b0a4..5fcbbdd449 100644 --- a/Library/Homebrew/livecheck/strategy/xorg.rb +++ b/Library/Homebrew/livecheck/strategy/xorg.rb @@ -64,6 +64,7 @@ module Homebrew # # @param url [String] the URL to match against # @return [Boolean] + sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) end From c936a9420eed710d955346cbfb76140a24d1b551 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Tue, 10 Aug 2021 18:38:21 -0400 Subject: [PATCH 5/6] Improve documentation comments --- Library/Homebrew/livecheck/strategy.rb | 32 ++++++++++++------- .../livecheck/strategy/electron_builder.rb | 10 +++--- .../livecheck/strategy/extract_plist.rb | 23 +++++++------ Library/Homebrew/livecheck/strategy/git.rb | 2 +- .../livecheck/strategy/header_match.rb | 21 +++++++----- .../Homebrew/livecheck/strategy/page_match.rb | 18 ++++++----- .../Homebrew/livecheck/strategy/sparkle.rb | 19 +++++++---- 7 files changed, 77 insertions(+), 48 deletions(-) diff --git a/Library/Homebrew/livecheck/strategy.rb b/Library/Homebrew/livecheck/strategy.rb index b77610d0c4..515cdb80a1 100644 --- a/Library/Homebrew/livecheck/strategy.rb +++ b/Library/Homebrew/livecheck/strategy.rb @@ -14,7 +14,7 @@ module Homebrew module_function - # Strategy priorities informally range from 1 to 10, where 10 is the + # {Strategy} priorities informally range from 1 to 10, where 10 is the # highest priority. 5 is the default priority because it's roughly in # the middle of this range. Strategies with a priority of 0 (or lower) # are ignored. @@ -32,10 +32,10 @@ module Homebrew # The `curl` process will sometimes hang indefinitely (despite setting # the `--max-time` argument) and it needs to be quit for livecheck to # continue. This value is used to set the `timeout` argument on - # `Utils::Curl` method calls in `Strategy`. + # `Utils::Curl` method calls in {Strategy}. CURL_PROCESS_TIMEOUT = CURL_MAX_TIME + 5 - # Baseline `curl` arguments used in `Strategy` methods. + # Baseline `curl` arguments used in {Strategy} methods. DEFAULT_CURL_ARGS = [ # Follow redirections to handle mirrors, relocations, etc. "--location", @@ -60,7 +60,7 @@ module Homebrew "--include", ] + DEFAULT_CURL_ARGS).freeze - # Baseline `curl` options used in `Strategy` methods. + # Baseline `curl` options used in {Strategy} methods. DEFAULT_CURL_OPTIONS = { print_stdout: false, print_stderr: false, @@ -81,7 +81,7 @@ module Homebrew # Creates and/or returns a `@strategies` `Hash`, which maps a snake # case strategy name symbol (e.g. `:page_match`) to the associated - # {Strategy}. + # strategy. # # At present, this should only be called after tap strategies have been # loaded, otherwise livecheck won't be able to use them. @@ -102,12 +102,12 @@ module Homebrew end private_class_method :strategies - # Returns the {Strategy} that corresponds to the provided `Symbol` (or - # `nil` if there is no matching {Strategy}). + # Returns the strategy that corresponds to the provided `Symbol` (or + # `nil` if there is no matching strategy). # - # @param symbol [Symbol] the strategy name in snake case as a `Symbol` - # (e.g. `:page_match`) - # @return [Strategy, nil] + # @param symbol [Symbol, nil] the strategy name in snake case as a + # `Symbol` (e.g. `:page_match`) + # @return [Class, nil] sig { params(symbol: T.nilable(Symbol)).returns(T.nilable(T.untyped)) } def from_symbol(symbol) strategies[symbol] if symbol.present? @@ -116,10 +116,14 @@ module Homebrew # Returns an array of strategies that apply to the provided URL. # # @param url [String] the URL to check for matching strategies - # @param livecheck_strategy [Symbol] a {Strategy} symbol from the + # @param livecheck_strategy [Symbol] a strategy symbol from the + # `livecheck` block + # @param url_provided [Boolean] whether a url is provided in the # `livecheck` block # @param regex_provided [Boolean] whether a regex is provided in the # `livecheck` block + # @param block_provided [Boolean] whether a `strategy` block is provided + # in the `livecheck` block # @return [Array] sig { params( @@ -154,6 +158,12 @@ module Homebrew end end + # Collects HTTP response headers, starting with the provided URL. + # Redirections will be followed and all the response headers are + # collected into an array of hashes. + # + # @param url [String] the URL to fetch + # @return [Array] sig { params(url: String).returns(T::Array[T::Hash[String, String]]) } def self.page_headers(url) headers = [] diff --git a/Library/Homebrew/livecheck/strategy/electron_builder.rb b/Library/Homebrew/livecheck/strategy/electron_builder.rb index 5c27824235..62baf90c1c 100644 --- a/Library/Homebrew/livecheck/strategy/electron_builder.rb +++ b/Library/Homebrew/livecheck/strategy/electron_builder.rb @@ -7,6 +7,9 @@ module Homebrew # The {ElectronBuilder} strategy fetches content at a URL and parses # it as an electron-builder appcast in YAML format. # + # This strategy is not applied automatically and it's necessary to use + # `strategy :electron_builder` in a `livecheck` block to apply it. + # # @api private class ElectronBuilder extend T::Sig @@ -14,8 +17,7 @@ module Homebrew NICE_NAME = "electron-builder" # A priority of zero causes livecheck to skip the strategy. We do this - # for {ElectronBuilder} so we can selectively apply the strategy using - # `strategy :electron_builder` in a `livecheck` block. + # for {ElectronBuilder} so we can selectively apply it when appropriate. PRIORITY = 0 # The `Regexp` used to determine if the strategy applies to the URL. @@ -54,10 +56,10 @@ module Homebrew version.present? ? [version] : [] end - # Checks the content at the URL for new versions. + # Checks the YAML content at the URL for new versions. # # @param url [String] the URL of the content to check - # @param regex [Regexp] a regex used for matching versions in content + # @param regex [Regexp, nil] a regex used for matching versions # @return [Hash] sig { params( diff --git a/Library/Homebrew/livecheck/strategy/extract_plist.rb b/Library/Homebrew/livecheck/strategy/extract_plist.rb index c9a12b0ce6..81ad951c1a 100644 --- a/Library/Homebrew/livecheck/strategy/extract_plist.rb +++ b/Library/Homebrew/livecheck/strategy/extract_plist.rb @@ -8,26 +8,31 @@ require_relative "page_match" module Homebrew module Livecheck module Strategy - # The {ExtractPlist} strategy downloads the file at a URL and - # extracts versions from contained `.plist` files. + # The {ExtractPlist} strategy downloads the file at a URL and extracts + # versions from contained `.plist` files using {UnversionedCaskChecker}. + # + # In practice, this strategy operates by downloading very large files, + # so it's both slow and data-intensive. As such, the {ExtractPlist} + # strategy should only be used as an absolute last resort. + # + # This strategy is not applied automatically and it's necessary to use + # `strategy :extract_plist` in a `livecheck` block to apply it. # # @api private class ExtractPlist extend T::Sig - # A priority of zero causes livecheck to skip the strategy. We only - # apply {ExtractPlist} using `strategy :extract_plist` in a `livecheck` block, - # as we can't automatically determine when this can be successfully - # applied to a URL without fetching the content. + # A priority of zero causes livecheck to skip the strategy. We do this + # for {ExtractPlist} so we can selectively apply it when appropriate. PRIORITY = 0 # The `Regexp` used to determine if the strategy applies to the URL. URL_MATCH_REGEX = %r{^https?://}i.freeze # Whether the strategy can be applied to the provided URL. - # The strategy will technically match any HTTP URL but is - # only usable with a `livecheck` block containing a regex - # or block. + # + # @param url [String] the URL to match against + # @return [Boolean] sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) diff --git a/Library/Homebrew/livecheck/strategy/git.rb b/Library/Homebrew/livecheck/strategy/git.rb index d68853ab9b..b4b7510199 100644 --- a/Library/Homebrew/livecheck/strategy/git.rb +++ b/Library/Homebrew/livecheck/strategy/git.rb @@ -118,7 +118,7 @@ module Homebrew # strings and parses the remaining text as a {Version}. # # @param url [String] the URL of the Git repository to check - # @param regex [Regexp] the regex to use for matching versions + # @param regex [Regexp, nil] a regex used for matching versions # @return [Hash] sig { params( diff --git a/Library/Homebrew/livecheck/strategy/header_match.rb b/Library/Homebrew/livecheck/strategy/header_match.rb index 66d7bccfbe..d1d2d36f3c 100644 --- a/Library/Homebrew/livecheck/strategy/header_match.rb +++ b/Library/Homebrew/livecheck/strategy/header_match.rb @@ -9,16 +9,17 @@ module Homebrew # The {HeaderMatch} strategy follows all URL redirections and scans # the resulting headers for matching text using the provided regex. # + # This strategy is not applied automatically and it's necessary to use + # `strategy :header_match` in a `livecheck` block to apply it. + # # @api private class HeaderMatch extend T::Sig NICE_NAME = "Header match" - # A priority of zero causes livecheck to skip the strategy. We only - # apply {HeaderMatch} using `strategy :header_match` in a `livecheck` - # block, as we can't automatically determine when this can be - # successfully applied to a URL. + # A priority of zero causes livecheck to skip the strategy. We do this + # for {HeaderMatch} so we can selectively apply it when appropriate. PRIORITY = 0 # The `Regexp` used to determine if the strategy applies to the URL. @@ -28,9 +29,9 @@ module Homebrew DEFAULT_HEADERS_TO_CHECK = ["content-disposition", "location"].freeze # Whether the strategy can be applied to the provided URL. - # The strategy will technically match any HTTP URL but is - # only usable with a `livecheck` block containing a regex - # or block. + # + # @param url [String] the URL to match against + # @return [Boolean] sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) @@ -39,7 +40,7 @@ module Homebrew # Identify versions from HTTP headers. # # @param headers [Hash] a hash of HTTP headers to check for versions - # @param regex [Regexp, nil] a regex to use to identify versions + # @param regex [Regexp, nil] a regex for matching versions # @return [Array] sig { params( @@ -71,6 +72,10 @@ module Homebrew # Checks the final URL for new versions after following all redirections, # using the provided regex for matching. + # + # @param url [String] the URL to fetch + # @param regex [Regexp, nil] a regex used for matching versions + # @return [Hash] sig { params( url: String, diff --git a/Library/Homebrew/livecheck/strategy/page_match.rb b/Library/Homebrew/livecheck/strategy/page_match.rb index 873d54a4fa..3b5d17f6d9 100644 --- a/Library/Homebrew/livecheck/strategy/page_match.rb +++ b/Library/Homebrew/livecheck/strategy/page_match.rb @@ -11,9 +11,8 @@ module Homebrew # strategies apply to a given URL. Though {PageMatch} will technically # match any HTTP URL, the strategy also requires a regex to function. # - # The {find_versions} method is also used within other - # strategies, to handle the process of identifying version text in - # content. + # The {find_versions} method is also used within other strategies, + # to handle the process of identifying version text in content. # # @api public class PageMatch @@ -22,16 +21,19 @@ module Homebrew NICE_NAME = "Page match" # A priority of zero causes livecheck to skip the strategy. We do this - # for PageMatch so we can selectively apply the strategy only when a - # regex is provided in a `livecheck` block. + # for {PageMatch} so we can selectively apply it only when a regex is + # provided in a `livecheck` block. PRIORITY = 0 # The `Regexp` used to determine if the strategy applies to the URL. URL_MATCH_REGEX = %r{^https?://}i.freeze # Whether the strategy can be applied to the provided URL. - # PageMatch will technically match any HTTP URL but is only + # {PageMatch} will technically match any HTTP URL but is only # usable with a `livecheck` block containing a regex. + # + # @param url [String] the URL to match against + # @return [Boolean] sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) @@ -71,8 +73,8 @@ module Homebrew # regex for matching. # # @param url [String] the URL of the content to check - # @param regex [Regexp] a regex used for matching versions in content - # @param provided_content [String] page content to use in place of + # @param regex [Regexp] a regex used for matching versions + # @param provided_content [String, nil] page content to use in place of # fetching via Strategy#page_content # @return [Hash] sig { diff --git a/Library/Homebrew/livecheck/strategy/sparkle.rb b/Library/Homebrew/livecheck/strategy/sparkle.rb index 9e7cf4f1cf..42b0460c9a 100644 --- a/Library/Homebrew/livecheck/strategy/sparkle.rb +++ b/Library/Homebrew/livecheck/strategy/sparkle.rb @@ -9,23 +9,24 @@ module Homebrew # The {Sparkle} strategy fetches content at a URL and parses # it as a Sparkle appcast in XML format. # + # This strategy is not applied automatically and it's necessary to use + # `strategy :sparkle` in a `livecheck` block to apply it. + # # @api private class Sparkle extend T::Sig - # A priority of zero causes livecheck to skip the strategy. We only - # apply {Sparkle} using `strategy :sparkle` in a `livecheck` block, - # as we can't automatically determine when this can be successfully - # applied to a URL without fetching the content. + # A priority of zero causes livecheck to skip the strategy. We do this + # for {Sparkle} so we can selectively apply it when appropriate. PRIORITY = 0 # The `Regexp` used to determine if the strategy applies to the URL. URL_MATCH_REGEX = %r{^https?://}i.freeze # Whether the strategy can be applied to the provided URL. - # The strategy will technically match any HTTP URL but is - # only usable with a `livecheck` block containing a regex - # or block. + # + # @param url [String] the URL to match against + # @return [Boolean] sig { params(url: String).returns(T::Boolean) } def self.match?(url) URL_MATCH_REGEX.match?(url) @@ -54,6 +55,10 @@ module Homebrew delegate short_version: :bundle_version end + # Identify version information from a Sparkle appcast. + # + # @param content [String] the text of the Sparkle appcast + # @return [Item, nil] sig { params(content: String).returns(T.nilable(Item)) } def self.item_from_content(content) require "rexml/document" From c59d5db6313f1478f51e0c8dd71b376566c67c75 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Tue, 10 Aug 2021 18:39:28 -0400 Subject: [PATCH 6/6] Remove unnecessary require statements --- Library/Homebrew/livecheck/strategy/extract_plist.rb | 1 - Library/Homebrew/livecheck/strategy/header_match.rb | 2 -- 2 files changed, 3 deletions(-) diff --git a/Library/Homebrew/livecheck/strategy/extract_plist.rb b/Library/Homebrew/livecheck/strategy/extract_plist.rb index 81ad951c1a..cb1b18e547 100644 --- a/Library/Homebrew/livecheck/strategy/extract_plist.rb +++ b/Library/Homebrew/livecheck/strategy/extract_plist.rb @@ -3,7 +3,6 @@ require "bundle_version" require "unversioned_cask_checker" -require_relative "page_match" module Homebrew module Livecheck diff --git a/Library/Homebrew/livecheck/strategy/header_match.rb b/Library/Homebrew/livecheck/strategy/header_match.rb index d1d2d36f3c..dfb34c344e 100644 --- a/Library/Homebrew/livecheck/strategy/header_match.rb +++ b/Library/Homebrew/livecheck/strategy/header_match.rb @@ -1,8 +1,6 @@ # typed: true # frozen_string_literal: true -require_relative "page_match" - module Homebrew module Livecheck module Strategy