From c6907f911f12cff4374d78a6f080721675e1d232 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Thu, 2 Jun 2022 10:04:34 -0400 Subject: [PATCH 1/2] Sparkle: Provide channel information in Item Making `channel` information available in the `Item` is necessary to be able to filter out unstable items using a `strategy` block. If an item doesn't specify a channel, then it uses the default channel (this is what Sparkle itself uses for updates). Channels like `beta` are something we want to avoid for stable casks and this allows for that type of [cask-specific] filtering. It's technically possible to automatically filter out items that aren't using the default channel (i.e., `channel != nil`) in `#items_from_content` but some casks use an unstable version, so we can't do this internally. That is to say, we wouldn't be able to override internal filtering in a `strategy` block, as any omitted items wouldn't be provided to the block. Conversely, if we pass all items to a `strategy` block, we can easily filter by channel there. We haven't been filtering by channel internally and we've only found one cask where this has been a problem, so it seems fine for now. --- .../Homebrew/livecheck/strategy/sparkle.rb | 4 ++++ .../test/livecheck/strategy/sparkle_spec.rb | 24 ++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/livecheck/strategy/sparkle.rb b/Library/Homebrew/livecheck/strategy/sparkle.rb index ac17dd344a..f599020f51 100644 --- a/Library/Homebrew/livecheck/strategy/sparkle.rb +++ b/Library/Homebrew/livecheck/strategy/sparkle.rb @@ -36,6 +36,8 @@ module Homebrew Item = Struct.new( # @api public :title, + # @api public + :channel, # @api private :pub_date, # @api public @@ -102,6 +104,7 @@ module Homebrew os = enclosure["os"] end + channel = item.elements["channel"]&.text url ||= item.elements["link"]&.text short_version ||= item.elements["shortVersionString"]&.text&.strip version ||= item.elements["version"]&.text&.strip @@ -135,6 +138,7 @@ module Homebrew data = { title: title, + channel: channel, pub_date: pub_date, url: url, bundle_version: bundle_version, diff --git a/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb b/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb index b850634300..8962d08d0a 100644 --- a/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb @@ -75,6 +75,13 @@ describe Homebrew::Livecheck::Strategy::Sparkle do EOS appcast_with_omitted_items = appcast_xml.sub("", "\n#{extra_items}") + beta_channel_item = appcast_xml.sub( + first_item, + first_item.sub( + "\nbeta", + ), + ) no_versions_item = appcast_xml .sub(second_item, "") @@ -89,6 +96,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do { appcast: appcast_xml, appcast_with_omitted_items: appcast_with_omitted_items, + beta_channel_item: beta_channel_item, no_versions_item: no_versions_item, no_items: no_items, undefined_namespace: undefined_namespace_xml, @@ -98,7 +106,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do let(:title_regex) { /Version\s+v?(\d+(?:\.\d+)+)\s*$/i } let(:items) { - { + items = { appcast: [ Homebrew::Livecheck::Strategy::Sparkle::Item.new( title: item_hash[0][:title], @@ -122,6 +130,12 @@ describe Homebrew::Livecheck::Strategy::Sparkle do ), ], } + + beta_channel_item = items[:appcast][0].clone + beta_channel_item.channel = "beta" + items[:beta_channel_item] = [beta_channel_item, items[:appcast][1].clone] + + items } let(:versions) { [items[:appcast][0].nice_version] } @@ -152,6 +166,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do expect(first_item.short_version).to eq(item_hash[0][:short_version]) expect(first_item.version).to eq(item_hash[0][:version]) + expect(sparkle.items_from_content(xml[:beta_channel_item])).to eq(items[:beta_channel_item]) expect(sparkle.items_from_content(xml[:no_versions_item])).to eq(items[:no_versions_item]) end end @@ -162,6 +177,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do it "returns an array of version strings when given content" do expect(sparkle.versions_from_content(xml[:appcast])).to eq(versions) expect(sparkle.versions_from_content(xml[:appcast_with_omitted_items])).to eq(versions) + expect(sparkle.versions_from_content(xml[:beta_channel_item])).to eq(versions) expect(sparkle.versions_from_content(xml[:no_versions_item])).to eq([]) expect(sparkle.versions_from_content(xml[:undefined_namespace])).to eq(versions) end @@ -184,6 +200,12 @@ describe Homebrew::Livecheck::Strategy::Sparkle do items.map { |item| item.nice_version&.sub("1", "0") } end, ).to eq(subbed_items) + + expect( + sparkle.versions_from_content(xml[:beta_channel_item]) do |items| + items.find { |item| item.channel.nil? }&.nice_version + end, + ).to eq([items[:appcast][1].nice_version]) end it "returns an array of version strings when given content, a regex, and a block" do From 71906c7f89ecdc22470676aa754c455282a879d3 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Thu, 2 Jun 2022 10:36:33 -0400 Subject: [PATCH 2/2] Tidy up Sparkle tests a bit --- .../test/livecheck/strategy/sparkle_spec.rb | 71 +++++++++---------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb b/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb index 8962d08d0a..74c533af14 100644 --- a/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb @@ -52,7 +52,15 @@ describe Homebrew::Livecheck::Strategy::Sparkle do EOS - appcast_xml = <<~EOS + items_to_omit = <<~EOS + #{first_item.sub(%r{<(enclosure[^>]+?)\s*?/>}, '<\1 os="not-osx" />')} + #{first_item.sub(/()[^<]+?)[^<]+? + + EOS + + appcast = <<~EOS @@ -66,16 +74,8 @@ describe Homebrew::Livecheck::Strategy::Sparkle do EOS - extra_items = <<~EOS - #{first_item.sub(%r{<(enclosure[^>]+?)\s*?/>}, '<\1 os="not-osx" />')} - #{first_item.sub(/()[^<]+?)[^<]+? - - EOS - - appcast_with_omitted_items = appcast_xml.sub("", "\n#{extra_items}") - beta_channel_item = appcast_xml.sub( + omitted_items = appcast.sub("", "\n#{items_to_omit}") + beta_channel_item = appcast.sub( first_item, first_item.sub( "#{item_hash[0][:title]}", "Version", ) - no_items = appcast_xml.sub(%r{.+}m, "") - undefined_namespace_xml = appcast_xml.sub(/\s*xmlns:sparkle="[^"]+?"/, "") + no_items = appcast.sub(%r{.+}m, "") + undefined_namespace = appcast.sub(/\s*xmlns:sparkle="[^"]+?"/, "") { - appcast: appcast_xml, - appcast_with_omitted_items: appcast_with_omitted_items, - beta_channel_item: beta_channel_item, - no_versions_item: no_versions_item, - no_items: no_items, - undefined_namespace: undefined_namespace_xml, + appcast: appcast, + omitted_items: omitted_items, + beta_channel_item: beta_channel_item, + no_versions_item: no_versions_item, + no_items: no_items, + undefined_namespace: undefined_namespace, } } @@ -107,7 +107,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do let(:items) { items = { - appcast: [ + appcast: [ Homebrew::Livecheck::Strategy::Sparkle::Item.new( title: item_hash[0][:title], pub_date: Time.parse(item_hash[0][:pub_date]), @@ -121,20 +121,17 @@ describe Homebrew::Livecheck::Strategy::Sparkle do bundle_version: Homebrew::BundleVersion.new(item_hash[1][:short_version], item_hash[1][:version]), ), ], - no_versions_item: [ - Homebrew::Livecheck::Strategy::Sparkle::Item.new( - title: "Version", - pub_date: Time.parse(item_hash[0][:pub_date]), - url: item_hash[0][:url], - bundle_version: nil, - ), - ], } beta_channel_item = items[:appcast][0].clone beta_channel_item.channel = "beta" items[:beta_channel_item] = [beta_channel_item, items[:appcast][1].clone] + no_versions_item = items[:appcast][0].clone + no_versions_item.title = "Version" + no_versions_item.bundle_version = nil + items[:no_versions_item] = [no_versions_item] + items } @@ -151,15 +148,15 @@ describe Homebrew::Livecheck::Strategy::Sparkle do end describe "::items_from_content" do - let(:items_from_appcast_xml) { sparkle.items_from_content(xml[:appcast]) } - let(:first_item) { items_from_appcast_xml[0] } + let(:items_from_appcast) { sparkle.items_from_content(xml[:appcast]) } + let(:first_item) { items_from_appcast[0] } it "returns nil if content is blank" do expect(sparkle.items_from_content("")).to eq([]) end it "returns an array of Items when given XML data" do - expect(items_from_appcast_xml).to eq(items[:appcast]) + expect(items_from_appcast).to eq(items[:appcast]) expect(first_item.title).to eq(item_hash[0][:title]) expect(first_item.pub_date).to eq(Time.parse(item_hash[0][:pub_date])) expect(first_item.url).to eq(item_hash[0][:url]) @@ -176,7 +173,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do it "returns an array of version strings when given content" do expect(sparkle.versions_from_content(xml[:appcast])).to eq(versions) - expect(sparkle.versions_from_content(xml[:appcast_with_omitted_items])).to eq(versions) + expect(sparkle.versions_from_content(xml[:omitted_items])).to eq(versions) expect(sparkle.versions_from_content(xml[:beta_channel_item])).to eq(versions) expect(sparkle.versions_from_content(xml[:no_versions_item])).to eq([]) expect(sparkle.versions_from_content(xml[:undefined_namespace])).to eq(versions) @@ -214,7 +211,7 @@ describe Homebrew::Livecheck::Strategy::Sparkle do sparkle.versions_from_content(xml[:appcast], title_regex) do |item, regex| item.title[regex, 1] end, - ).to eq([items[:appcast][0].short_version]) + ).to eq([item_hash[0][:short_version]]) expect( sparkle.versions_from_content(xml[:appcast], title_regex) do |items, regex| @@ -225,18 +222,18 @@ describe Homebrew::Livecheck::Strategy::Sparkle do "#{match[1]},#{item.version}" end, - ).to eq(["#{items[:appcast][0].short_version},#{items[:appcast][0].version}"]) + ).to eq(["#{item_hash[0][:short_version]},#{item_hash[0][:version]}"]) # Returning an array of strings from the block expect( sparkle.versions_from_content(xml[:appcast], title_regex) do |item, regex| [item.title[regex, 1]] end, - ).to eq([items[:appcast][0].short_version]) + ).to eq([item_hash[0][:short_version]]) expect( sparkle.versions_from_content(xml[:appcast], &:short_version), - ).to eq([items[:appcast][0].short_version]) + ).to eq([item_hash[0][:short_version]]) expect( sparkle.versions_from_content(xml[:appcast], title_regex) do |items, regex|