bump-cask-pr: extract system_options generation

This extracts the logic for generating the `system_options` array in
the `replace_version_and_checksum` method into a separate
`generate_system_options` method. This logic is becoming more complex
(after recent changes) and manually testing it is a pain, so this
change is intended to allow us to add tests. The tests added here
provide 100% coverage for the method.

Library/Homebrew/dev-cmd/bump-cask-pr.rb

@@ -181,15 +181,8 @@ module Homebrew
         end
       end
 
-      sig {
+      sig { params(cask: Cask::Cask).returns(T::Array[[Symbol, Symbol]]) }
-        params(
+      def generate_system_options(cask)
-          cask:              Cask::Cask,
-          new_hash:          T.any(NilClass, String, Symbol),
-          new_version:       BumpVersionParser,
-          replacement_pairs: T::Array[[T.any(Regexp, String), T.any(Pathname, String)]],
-        ).returns(T::Array[[T.any(Regexp, String), T.any(Pathname, String)]])
-      }
-      def replace_version_and_checksum(cask, new_hash, new_version, replacement_pairs)
         host_os = Homebrew::SimulateSystem.current_os
         host_is_macos = MacOSVersion::SYMBOLS.include?(host_os)
         newest_macos = MacOSVersion::SYMBOLS.keys.first
@@ -219,7 +212,19 @@ module Homebrew
           arch_values << :arm if arch_values.empty?
         end
 
-        os_values.product(arch_values).each do |os, arch|
+        os_values.product(arch_values)
+      end
+
+      sig {
+        params(
+          cask:              Cask::Cask,
+          new_hash:          T.any(NilClass, String, Symbol),
+          new_version:       BumpVersionParser,
+          replacement_pairs: T::Array[[T.any(Regexp, String), T.any(Pathname, String)]],
+        ).returns(T::Array[[T.any(Regexp, String), T.any(Pathname, String)]])
+      }
+      def replace_version_and_checksum(cask, new_hash, new_version, replacement_pairs)
+        generate_system_options(cask).each do |os, arch|
           SimulateSystem.with(os:, arch:) do
             # Handle the cask being invalid for specific os/arch combinations
             old_cask = begin

Library/Homebrew/test/dev-cmd/bump-cask-pr_spec.rb

@@ -4,5 +4,139 @@ require "cmd/shared_examples/args_parse"
 require "dev-cmd/bump-cask-pr"
 
 RSpec.describe Homebrew::DevCmd::BumpCaskPr do
+  subject(:bump_cask_pr) { described_class.new(["test"]) }
+
+  let(:newest_macos) { MacOSVersion::SYMBOLS.keys.first }
+
+  let(:c) do
+    Cask::Cask.new("test") do
+      version "0.0.1,2"
+
+      url "https://brew.sh/test-0.0.1.dmg"
+      name "Test"
+      desc "Test cask"
+      homepage "https://brew.sh"
+    end
+  end
+
+  let(:c_depends_on_intel) do
+    Cask::Cask.new("test-depends-on-intel") do
+      version "0.0.1,2"
+
+      url "https://brew.sh/test-0.0.1.dmg"
+      name "Test"
+      desc "Test cask"
+      homepage "https://brew.sh"
+
+      depends_on arch: :x86_64
+    end
+  end
+
+  let(:c_on_system) do
+    Cask::Cask.new("test-on-system") do
+      os macos: "darwin", linux: "linux"
+
+      version "0.0.1,2"
+
+      url "https://brew.sh/test-0.0.1.dmg"
+      name "Test"
+      desc "Test cask"
+      homepage "https://brew.sh"
+    end
+  end
+
+  let(:c_on_system_depends_on_intel) do
+    Cask::Cask.new("test-on-system-depends-on-intel") do
+      os macos: "darwin", linux: "linux"
+
+      version "0.0.1,2"
+
+      url "https://brew.sh/test-0.0.1.dmg"
+      name "Test"
+      desc "Test cask"
+      homepage "https://brew.sh"
+
+      depends_on arch: :x86_64
+    end
+  end
+
   it_behaves_like "parseable arguments"
+
+  describe "::generate_system_options" do
+    # We simulate a macOS version older than the newest, as the method will use
+    # the host macOS version instead of the default (the newest macOS version).
+    let(:older_macos) { :big_sur }
+
+    context "when cask does not have on_system blocks/calls or `depends_on arch`" do
+      it "returns an array only including macOS/ARM" do
+        Homebrew::SimulateSystem.with(os: :linux) do
+          expect(bump_cask_pr.send(:generate_system_options, c))
+            .to eq([[newest_macos, :arm]])
+        end
+
+        Homebrew::SimulateSystem.with(os: older_macos) do
+          expect(bump_cask_pr.send(:generate_system_options, c))
+            .to eq([[older_macos, :arm]])
+        end
+      end
+    end
+
+    context "when cask does not have on_system blocks/calls but has `depends_on arch`" do
+      it "returns an array only including macOS/`depends_on arch` value" do
+        Homebrew::SimulateSystem.with(os: :linux, arch: :arm) do
+          expect(bump_cask_pr.send(:generate_system_options, c_depends_on_intel))
+            .to eq([[newest_macos, :intel]])
+        end
+
+        Homebrew::SimulateSystem.with(os: older_macos, arch: :arm) do
+          expect(bump_cask_pr.send(:generate_system_options, c_depends_on_intel))
+            .to eq([[older_macos, :intel]])
+        end
+      end
+    end
+
+    context "when cask has on_system blocks/calls but does not have `depends_on arch`" do
+      it "returns an array with combinations of `OnSystem::BASE_OS_OPTIONS` and `OnSystem::ARCH_OPTIONS`" do
+        Homebrew::SimulateSystem.with(os: :linux) do
+          expect(bump_cask_pr.send(:generate_system_options, c_on_system))
+            .to eq([
+              [newest_macos, :intel],
+              [newest_macos, :arm],
+              [:linux, :intel],
+              [:linux, :arm],
+            ])
+        end
+
+        Homebrew::SimulateSystem.with(os: older_macos) do
+          expect(bump_cask_pr.send(:generate_system_options, c_on_system))
+            .to eq([
+              [older_macos, :intel],
+              [older_macos, :arm],
+              [:linux, :intel],
+              [:linux, :arm],
+            ])
+        end
+      end
+    end
+
+    context "when cask has on_system blocks/calls and `depends_on arch`" do
+      it "returns an array with combinations of `OnSystem::BASE_OS_OPTIONS` and `depends_on arch` value" do
+        Homebrew::SimulateSystem.with(os: :linux, arch: :arm) do
+          expect(bump_cask_pr.send(:generate_system_options, c_on_system_depends_on_intel))
+            .to eq([
+              [newest_macos, :intel],
+              [:linux, :intel],
+            ])
+        end
+
+        Homebrew::SimulateSystem.with(os: older_macos, arch: :arm) do
+          expect(bump_cask_pr.send(:generate_system_options, c_on_system_depends_on_intel))
+            .to eq([
+              [older_macos, :intel],
+              [:linux, :intel],
+            ])
+        end
+      end
+    end
+  end
 end