Skip to content

Commit 7dc20cd

Browse files
committed
ExtractPlist: expand tests
This extracts the logic for creating a duplicate cask with a different artifact URL into a separate method, so we can properly test it. It's possible to exercise this in `ExtractPlist.find_versions` tests to some degree but isolating this code allows us to check the returned cask and ensure that this works like it should. This also expands `ExtractPlist` tests, bringing coverage to 100% for both lines and branches. I removed an existing `find_versions` test case which seemed to only check that `Cask::CaskLoader.load` works, as it wasn't testing `ExtractPlist`. Similarly, I removed the `UnversionedCaskChecker` expectations, as these tests should arguably be checking the output from `ExtractPlist` methods, not what's happening inside.
1 parent f02a48d commit 7dc20cd

File tree

6 files changed

+187
-52
lines changed

6 files changed

+187
-52
lines changed

Library/Homebrew/livecheck/strategy/extract_plist.rb

Lines changed: 54 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,59 @@ def self.versions_from_items(items, regex = nil, &block)
7575
end.uniq
7676
end
7777

78+
# Creates a copy of the cask with the artifact URL replaced by the
79+
# provided URL, using the provided `url_options`. This will error if
80+
# `url_options` contains any non-nil values with keys that aren't
81+
# found in the `Cask::URL.initialize` keyword parameters.
82+
# @param cask [Cask::Cask] the cask to copy and modify to use the
83+
# provided URL and options
84+
# @param url [String] the replacement URL
85+
# @param url_options [Hash] options to use when replacing the URL
86+
# @return [Cask::Cask]
87+
sig {
88+
params(
89+
cask: Cask::Cask,
90+
url: String,
91+
url_options: T::Hash[Symbol, T.untyped],
92+
).returns(Cask::Cask)
93+
}
94+
def self.cask_with_url(cask, url, url_options)
95+
# Collect the `Cask::URL` initializer keyword parameter symbols
96+
@cask_url_kw_params ||= T.let(
97+
T::Utils.signature_for_method(
98+
Cask::URL.instance_method(:initialize),
99+
).parameters.filter_map { |type, sym| sym if type == :key },
100+
T.nilable(T::Array[Symbol]),
101+
)
102+
103+
# Collect `livecheck` block URL options supported by `Cask::URL`
104+
unused_opts = []
105+
url_kwargs = url_options.select do |key, value|
106+
next if value.nil?
107+
108+
unless @cask_url_kw_params.include?(key)
109+
unused_opts << key
110+
next
111+
end
112+
113+
true
114+
end
115+
116+
unless unused_opts.empty?
117+
raise ArgumentError,
118+
"Cask `url` does not support `#{unused_opts.join("`, `")}` " \
119+
"#{Utils.pluralize("option", unused_opts.length)} from " \
120+
"`livecheck` block"
121+
end
122+
123+
# Create a copy of the cask that overrides the artifact URL with the
124+
# provided URL and supported `livecheck` block URL options
125+
cask_copy = Cask::CaskLoader.load(cask.sourcefile_path)
126+
cask_copy.allow_reassignment = true
127+
cask_copy.url(url, **url_kwargs)
128+
cask_copy
129+
end
130+
78131
# Uses {UnversionedCaskChecker} on the provided cask to identify
79132
# versions from `plist` files.
80133
#
@@ -98,46 +151,11 @@ def self.find_versions(cask:, url: nil, regex: nil, options: Options.new, &block
98151
raise ArgumentError,
99152
"#{Utils.demodulize(name)} only supports a regex when using a `strategy` block"
100153
end
101-
raise ArgumentError, "The #{Utils.demodulize(name)} strategy only supports casks." unless T.unsafe(cask)
102154

103155
match_data = { matches: {}, regex:, url: }
104156

105157
unversioned_cask_checker = if url.present? && url != cask.url.to_s
106-
# Collect the `Cask::URL` initializer keyword parameter symbols
107-
@cask_url_kw_params ||= T.let(
108-
T::Utils.signature_for_method(
109-
Cask::URL.instance_method(:initialize),
110-
).parameters.filter_map { |type, sym| sym if type == :key },
111-
T.nilable(T::Array[Symbol]),
112-
)
113-
114-
# Collect `livecheck` block URL options supported by `Cask::URL`
115-
unused_opts = []
116-
url_kwargs = options.url_options.select do |key, value|
117-
next if value.nil?
118-
119-
unless @cask_url_kw_params.include?(key)
120-
unused_opts << key
121-
next
122-
end
123-
124-
true
125-
end
126-
127-
unless unused_opts.empty?
128-
raise ArgumentError,
129-
"Cask `url` does not support `#{unused_opts.join("`, `")}` " \
130-
"#{Utils.pluralize("option", unused_opts.length)} from " \
131-
"`livecheck` block"
132-
end
133-
134-
# Create a copy of the cask that overrides the artifact URL with the
135-
# provided URL and supported `livecheck` block URL options
136-
cask_copy = Cask::CaskLoader.load(cask.sourcefile_path)
137-
cask_copy.allow_reassignment = true
138-
cask_copy.url(url, **url_kwargs)
139-
140-
UnversionedCaskChecker.new(cask_copy)
158+
UnversionedCaskChecker.new(cask_with_url(cask, url, options.url_options))
141159
else
142160
UnversionedCaskChecker.new(cask)
143161
end

Library/Homebrew/test/livecheck/strategy/extract_plist_spec.rb

Lines changed: 75 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,23 +106,86 @@
106106
end
107107
end
108108

109+
describe "::cask_with_url" do
110+
it "returns a cask using the url and supported options from the `livecheck` block" do
111+
cask = Cask::CaskLoader.load(cask_path("livecheck/livecheck-extract-plist-with-url"))
112+
cask.livecheck.url(
113+
cask.livecheck.url,
114+
cookies: { "key" => "value" },
115+
header: "Origin: https://example.com",
116+
referer: "https://example.com/referer",
117+
user_agent: :browser,
118+
)
119+
livecheck_url = cask.livecheck.url
120+
url_options = cask.livecheck.options.url_options
121+
122+
returned_cask = extract_plist.cask_with_url(cask, livecheck_url, url_options)
123+
returned_cask_url = returned_cask.url
124+
125+
expect(returned_cask_url.to_s).to eq(livecheck_url)
126+
# NOTE: `Cask::URL` converts symbol keys to strings
127+
expect(returned_cask_url.cookies).to eq(url_options[:cookies].transform_keys(&:to_s))
128+
# NOTE: `Cask::URL` creates an array from a header string argument
129+
expect(returned_cask_url.header).to eq([url_options[:header]])
130+
expect(returned_cask_url.referer).to eq(url_options[:referer])
131+
expect(returned_cask_url.user_agent).to eq(url_options[:user_agent])
132+
end
133+
134+
it "errors if the `livecheck` block uses options not supported by `Cask::URL`" do
135+
cask = Cask::CaskLoader.load(cask_path("livecheck/livecheck-extract-plist-with-url"))
136+
livecheck_url = cask.livecheck.url
137+
cask.livecheck.url(
138+
livecheck_url,
139+
post_form: { key: "value" },
140+
user_agent: :browser,
141+
)
142+
options = cask.livecheck.options
143+
144+
expect do
145+
extract_plist.cask_with_url(cask, livecheck_url, options.url_options)
146+
end.to raise_error(
147+
ArgumentError,
148+
"Cask `url` does not support `post_form` option from `livecheck` block",
149+
)
150+
151+
options.homebrew_curl = true
152+
expect do
153+
extract_plist.cask_with_url(cask, livecheck_url, options.url_options)
154+
end.to raise_error(
155+
ArgumentError,
156+
"Cask `url` does not support `homebrew_curl`, `post_form` options from `livecheck` block",
157+
)
158+
end
159+
end
160+
109161
describe "::find_versions" do
110-
it "returns a for an installer artifact" do
111-
cask = Cask::CaskLoader.load(cask_path("livecheck/livecheck-installer-manual"))
112-
installer_artifact = cask.artifacts.first
162+
let(:cask) { Cask::CaskLoader.load(cask_path("livecheck/livecheck-extract-plist")) }
163+
let(:matches) { { "1.2.3" => Version.new("1.2.3") } }
113164

114-
expect(installer_artifact).to be_a(Cask::Artifact::Installer)
115-
expect(installer_artifact.path).to be_a(Pathname)
165+
it "raises an error if a regex is provided with no block" do
166+
expect do
167+
extract_plist.find_versions(cask:, regex: multipart_regex)
168+
end.to raise_error(ArgumentError, "ExtractPlist only supports a regex when using a `strategy` block")
116169
end
117170

118-
it "uses the provided livecheck url", :needs_macos do
119-
cask = Cask::CaskLoader.load(cask_path("livecheck/livecheck-extract-plist"))
120-
livecheck_url = "file://#{TEST_FIXTURE_DIR}/cask/caffeine-with-plist.zip"
171+
it "checks the cask using the livecheck URL string", :needs_macos do
172+
cask_with_url = Cask::CaskLoader.load(cask_path("livecheck/livecheck-extract-plist-with-url"))
173+
livecheck_url = cask_with_url.livecheck.url
174+
175+
expect(
176+
extract_plist.find_versions(cask: cask_with_url, url: livecheck_url),
177+
).to eq({ matches:, regex: nil, url: livecheck_url })
178+
end
179+
180+
it "checks the original cask if the provided URL is the same as the artifact URL", :needs_macos do
181+
cask_url = cask.url.to_s
182+
183+
expect(extract_plist.find_versions(cask:, url: cask_url))
184+
.to eq({ matches:, regex: nil, url: cask_url })
185+
end
121186

122-
expect(Homebrew::UnversionedCaskChecker).to receive(:new).with(cask).and_call_original
123-
result = described_class.find_versions(cask:, url: livecheck_url)
124-
expect(result)
125-
.to eq({ matches: { "1.2.3"=> @version="1.2.3" }, regex: nil, url: livecheck_url })
187+
it "checks the original cask if a URL is not provided", :needs_macos do
188+
expect(extract_plist.find_versions(cask:)).to eq({ matches:, regex: nil, url: nil })
126189
end
127190
end
128191
end
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
cask "livecheck-extract-plist-with-supported-url-options" do
2+
version "1.2.3"
3+
sha256 "78c670559a609f5d89a5d75eee49e2a2dab48aa3ea36906d14d5f7104e483bb9"
4+
5+
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine-suite.zip"
6+
name "ExtractPlist livecheck with supported URL options"
7+
desc "Cask with an ExtractPlist livecheck block using supported URL options"
8+
homepage "https://brew.sh/"
9+
10+
livecheck do
11+
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine-with-plist.zip",
12+
cookies: { "key" => "value" },
13+
header: "Origin: https://example.com",
14+
referer: "https://example.com/referer",
15+
user_agent: :browser
16+
strategy :extract_plist
17+
end
18+
19+
app "Caffeine.app"
20+
end
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
cask "livecheck-extract-plist-with-unsupported-url-options" do
2+
version "1.2.3"
3+
sha256 "78c670559a609f5d89a5d75eee49e2a2dab48aa3ea36906d14d5f7104e483bb9"
4+
5+
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine-suite.zip"
6+
name "ExtractPlist livecheck with unsupported URL options"
7+
desc "Cask with an ExtractPlist livecheck block using unsupported URL options"
8+
homepage "https://brew.sh/"
9+
10+
livecheck do
11+
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine-with-plist.zip",
12+
post_form: { key: "value" },
13+
user_agent: :browser
14+
strategy :extract_plist
15+
end
16+
17+
app "Caffeine.app"
18+
end
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
cask "livecheck-extract-plist-with-url" do
2+
version "1.2.3"
3+
sha256 "78c670559a609f5d89a5d75eee49e2a2dab48aa3ea36906d14d5f7104e483bb9"
4+
5+
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine-suite.zip"
6+
name "ExtractPlist livecheck with a URL string"
7+
desc "Cask with an ExtractPlist livecheck block using a URL string"
8+
homepage "https://brew.sh/"
9+
10+
livecheck do
11+
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine-with-plist.zip"
12+
strategy :extract_plist
13+
end
14+
15+
app "Caffeine.app"
16+
end

Library/Homebrew/test/support/fixtures/cask/Casks/livecheck/livecheck-extract-plist.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
version "1.2.3"
33
sha256 "78c670559a609f5d89a5d75eee49e2a2dab48aa3ea36906d14d5f7104e483bb9"
44

5-
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine-suite.zip"
6-
name "With Extract Plist Livecheck"
7-
desc "Cask with a an ExtractPlist livecheck strategy"
5+
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine-with-plist.zip"
6+
name "ExtractPlist livecheck"
7+
desc "Cask with an ExtractPlist livecheck block"
88
homepage "https://brew.sh/"
99

1010
livecheck do
11-
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine-with-plist.zip"
11+
url :url
1212
strategy :extract_plist
1313
end
1414

0 commit comments

Comments
 (0)