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] 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