From ca549fa10d89a8d220113440f42ec189513dc50f Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Fri, 17 Nov 2023 01:38:10 +0000 Subject: [PATCH 1/2] dev-cmd/determine-test-runner: add `--all-supported` --- .../dev-cmd/determine-test-runners.rb | 35 +++++---- Library/Homebrew/github_runner_matrix.rb | 28 ++++--- .../test/github_runner_matrix_spec.rb | 77 +++++++++++++------ 3 files changed, 93 insertions(+), 47 deletions(-) diff --git a/Library/Homebrew/dev-cmd/determine-test-runners.rb b/Library/Homebrew/dev-cmd/determine-test-runners.rb index 6e867c572e3f5..9a94f20ca9345 100755 --- a/Library/Homebrew/dev-cmd/determine-test-runners.rb +++ b/Library/Homebrew/dev-cmd/determine-test-runners.rb @@ -10,17 +10,23 @@ module Homebrew def self.determine_test_runners_args Homebrew::CLI::Parser.new do usage_banner <<~EOS - `determine-test-runners` [] + `determine-test-runners` { []|--all-supported} - Determines the runners used to test formulae or their dependents. + Determines the runners used to test formulae or their dependents. For internal use in Homebrew taps. EOS + switch "--all-supported", + description: "Instead of selecting runners based on the chosen formula, return all supported runners." switch "--eval-all", description: "Evaluate all available formulae, whether installed or not, to determine testing " \ - "dependents." + "dependents.", + env: :eval_all switch "--dependents", - description: "Determine runners for testing dependents. Requires `--eval-all` or `HOMEBREW_EVAL_ALL`." + description: "Determine runners for testing dependents. Requires `--eval-all` or `HOMEBREW_EVAL_ALL`.", + depends_on: "--eval-all" - named_args min: 1, max: 2 + named_args max: 2 + + conflicts "--all-supported", "--dependents" hide_from_man_page! end @@ -30,16 +36,19 @@ def self.determine_test_runners_args def self.determine_test_runners args = determine_test_runners_args.parse - eval_all = args.eval_all? || Homebrew::EnvConfig.eval_all? - - odie "`--dependents` requires `--eval-all` or `HOMEBREW_EVAL_ALL`!" if args.dependents? && !eval_all + if args.no_named? && !args.all_supported? + raise Homebrew::CLI::MinNamedArgumentsError, 1 + elsif args.all_supported? && !args.no_named? + raise UsageError, "`--all-supported` is mutually exclusive to other arguments." + end - testing_formulae = args.named.first.split(",") - testing_formulae.map! { |name| TestRunnerFormula.new(Formulary.factory(name), eval_all: eval_all) } + testing_formulae = args.named.first&.split(",").to_a + testing_formulae.map! { |name| TestRunnerFormula.new(Formulary.factory(name), eval_all: args.eval_all?) } .freeze - deleted_formulae = args.named.second&.split(",").freeze - - runner_matrix = GitHubRunnerMatrix.new(testing_formulae, deleted_formulae, dependent_matrix: args.dependents?) + deleted_formulae = args.named.second&.split(",").to_a.freeze + runner_matrix = GitHubRunnerMatrix.new(testing_formulae, deleted_formulae, + all_supported: args.all_supported?, + dependent_matrix: args.dependents?) runners = runner_matrix.active_runner_specs_hash ohai "Runners", JSON.pretty_generate(runners) diff --git a/Library/Homebrew/github_runner_matrix.rb b/Library/Homebrew/github_runner_matrix.rb index 7153e9f121daa..dd5b413334264 100644 --- a/Library/Homebrew/github_runner_matrix.rb +++ b/Library/Homebrew/github_runner_matrix.rb @@ -7,9 +7,6 @@ class GitHubRunnerMatrix # FIXME: Enable cop again when https://github.com/sorbet/sorbet/issues/3532 is fixed. # rubocop:disable Style/MutableConstant - MaybeStringArray = T.type_alias { T.nilable(T::Array[String]) } - private_constant :MaybeStringArray - RunnerSpec = T.type_alias { T.any(LinuxRunnerSpec, MacOSRunnerSpec) } private_constant :RunnerSpec @@ -38,13 +35,19 @@ class GitHubRunnerMatrix sig { params( testing_formulae: T::Array[TestRunnerFormula], - deleted_formulae: MaybeStringArray, + deleted_formulae: T::Array[String], + all_supported: T::Boolean, dependent_matrix: T::Boolean, ).void } - def initialize(testing_formulae, deleted_formulae, dependent_matrix:) + def initialize(testing_formulae, deleted_formulae, all_supported:, dependent_matrix:) + if all_supported && (testing_formulae.present? || deleted_formulae.present? || dependent_matrix) + raise ArgumentError, "all_supported is mutually exclusive to other arguments" + end + @testing_formulae = T.let(testing_formulae, T::Array[TestRunnerFormula]) - @deleted_formulae = T.let(deleted_formulae, MaybeStringArray) + @deleted_formulae = T.let(deleted_formulae, T::Array[String]) + @all_supported = T.let(all_supported, T::Boolean) @dependent_matrix = T.let(dependent_matrix, T::Boolean) @runners = T.let([], T::Array[GitHubRunner]) @@ -103,13 +106,16 @@ def create_runner(platform, arch, spec, macos_version = nil) end NEWEST_GITHUB_ACTIONS_MACOS_RUNNER = :ventura + OLDEST_GITHUB_ACTIONS_MACOS_RUNNER = :big_sur GITHUB_ACTIONS_RUNNER_TIMEOUT = 360 sig { void } def generate_runners! return if @runners.present? - @runners << create_runner(:linux, :x86_64, linux_runner_spec) + if !@all_supported || ENV.key?("HOMEBREW_LINUX_RUNNER") + @runners << create_runner(:linux, :x86_64, linux_runner_spec) + end github_run_id = ENV.fetch("GITHUB_RUN_ID") timeout = ENV.fetch("HOMEBREW_MACOS_TIMEOUT").to_i @@ -132,6 +138,7 @@ def generate_runners! # Use GitHub Actions macOS Runner for testing dependents if compatible with timeout. runner, runner_timeout = if (@dependent_matrix || use_github_runner) && macos_version <= NEWEST_GITHUB_ACTIONS_MACOS_RUNNER && + macos_version >= OLDEST_GITHUB_ACTIONS_MACOS_RUNNER && runner_timeout <= GITHUB_ACTIONS_RUNNER_TIMEOUT ["macos-#{version}", GITHUB_ACTIONS_RUNNER_TIMEOUT] else @@ -149,6 +156,7 @@ def generate_runners! next if macos_version < :big_sur runner = +"#{version}-arm64" + runner_timeout = timeout # Use bare metal runner when testing dependents on ARM64 Monterey. use_ephemeral = macos_version >= (@dependent_matrix ? :ventura : :monterey) @@ -174,9 +182,7 @@ def generate_runners! def active_runner?(runner) if @dependent_matrix formulae_have_untested_dependents?(runner) - else - return true if @deleted_formulae.present? - + elsif !@all_supported && @deleted_formulae.empty? compatible_formulae = @testing_formulae.dup platform = runner.platform @@ -191,6 +197,8 @@ def active_runner?(runner) end compatible_formulae.present? + else + true end end diff --git a/Library/Homebrew/test/github_runner_matrix_spec.rb b/Library/Homebrew/test/github_runner_matrix_spec.rb index 1f22194f685d6..07b5d147e8031 100644 --- a/Library/Homebrew/test/github_runner_matrix_spec.rb +++ b/Library/Homebrew/test/github_runner_matrix_spec.rb @@ -31,7 +31,7 @@ describe "#active_runner_specs_hash" do it "returns an object that responds to `#to_json`" do expect( - described_class.new([], ["deleted"], dependent_matrix: false) + described_class.new([], ["deleted"], all_supported: false, dependent_matrix: false) .active_runner_specs_hash .respond_to?(:to_json), ).to be(true) @@ -40,7 +40,7 @@ describe "#generate_runners!" do it "is idempotent" do - matrix = described_class.new([], [], dependent_matrix: false) + matrix = described_class.new([], [], all_supported: false, dependent_matrix: false) runners = matrix.runners.dup matrix.send(:generate_runners!) @@ -50,28 +50,39 @@ context "when there are no testing formulae and no deleted formulae" do it "activates no test runners" do - expect(described_class.new([], [], dependent_matrix: false).runners.any?(&:active)) + expect(described_class.new([], [], all_supported: false, dependent_matrix: false).runners.any?(&:active)) .to be(false) end it "activates no dependent runners" do - expect(described_class.new([], [], dependent_matrix: true).runners.any?(&:active)) + expect(described_class.new([], [], all_supported: false, dependent_matrix: true).runners.any?(&:active)) .to be(false) end end + context "when passed `--all-supported`" do + it "activates all runners" do + expect(described_class.new([], [], all_supported: true, dependent_matrix: false).runners.all?(&:active)) + .to be(true) + end + end + context "when there are testing formulae and no deleted formulae" do context "when it is a matrix for the `tests` job" do context "when testing formulae have no requirements" do it "activates all runners" do - expect(described_class.new([testball], [], dependent_matrix: false).runners.all?(&:active)) + expect(described_class.new([testball], [], all_supported: false, dependent_matrix: false) + .runners + .all?(&:active)) .to be(true) end end context "when testing formulae require Linux" do it "activates only the Linux runner" do - runner_matrix = described_class.new([testball_depender_linux], [], dependent_matrix: false) + runner_matrix = described_class.new([testball_depender_linux], [], + all_supported: false, + dependent_matrix: false) expect(runner_matrix.runners.all?(&:active)).to be(false) expect(runner_matrix.runners.any?(&:active)).to be(true) @@ -81,7 +92,9 @@ context "when testing formulae require macOS" do it "activates only the macOS runners" do - runner_matrix = described_class.new([testball_depender_macos], [], dependent_matrix: false) + runner_matrix = described_class.new([testball_depender_macos], [], + all_supported: false, + dependent_matrix: false) expect(runner_matrix.runners.all?(&:active)).to be(false) expect(runner_matrix.runners.any?(&:active)).to be(true) @@ -91,7 +104,9 @@ context "when testing formulae require Intel" do it "activates only the Intel runners" do - runner_matrix = described_class.new([testball_depender_intel], [], dependent_matrix: false) + runner_matrix = described_class.new([testball_depender_intel], [], + all_supported: false, + dependent_matrix: false) expect(runner_matrix.runners.all?(&:active)).to be(false) expect(runner_matrix.runners.any?(&:active)).to be(true) @@ -101,7 +116,9 @@ context "when testing formulae require ARM" do it "activates only the ARM runners" do - runner_matrix = described_class.new([testball_depender_arm], [], dependent_matrix: false) + runner_matrix = described_class.new([testball_depender_arm], [], + all_supported: false, + dependent_matrix: false) expect(runner_matrix.runners.all?(&:active)).to be(false) expect(runner_matrix.runners.any?(&:active)).to be(true) @@ -112,7 +129,9 @@ context "when testing formulae require a macOS version" do it "activates the Linux runner and suitable macOS runners" do _, v = newest_supported_macos - runner_matrix = described_class.new([testball_depender_newest], [], dependent_matrix: false) + runner_matrix = described_class.new([testball_depender_newest], [], + all_supported: false, + dependent_matrix: false) expect(runner_matrix.runners.all?(&:active)).to be(false) expect(runner_matrix.runners.any?(&:active)).to be(true) @@ -127,7 +146,9 @@ allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true) allow(Formula).to receive(:all).and_return([testball].map(&:formula)) - expect(described_class.new([testball], [], dependent_matrix: true).runners.any?(&:active)) + expect(described_class.new([testball], [], all_supported: false, dependent_matrix: true) + .runners + .any?(&:active)) .to be(false) end end @@ -138,7 +159,9 @@ allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true) allow(Formula).to receive(:all).and_return([testball, testball_depender].map(&:formula)) - expect(described_class.new([testball], [], dependent_matrix: true).runners.all?(&:active)) + expect(described_class.new([testball], [], all_supported: false, dependent_matrix: true) + .runners + .all?(&:active)) .to be(true) end end @@ -148,7 +171,7 @@ allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true) allow(Formula).to receive(:all).and_return([testball, testball_depender_linux].map(&:formula)) - runner_matrix = described_class.new([testball], [], dependent_matrix: true) + runner_matrix = described_class.new([testball], [], all_supported: false, dependent_matrix: true) expect(runner_matrix.runners.all?(&:active)).to be(false) expect(runner_matrix.runners.any?(&:active)).to be(true) expect(get_runner_names(runner_matrix)).to eq(get_runner_names(runner_matrix, :linux?)) @@ -160,7 +183,7 @@ allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true) allow(Formula).to receive(:all).and_return([testball, testball_depender_macos].map(&:formula)) - runner_matrix = described_class.new([testball], [], dependent_matrix: true) + runner_matrix = described_class.new([testball], [], all_supported: false, dependent_matrix: true) expect(runner_matrix.runners.all?(&:active)).to be(false) expect(runner_matrix.runners.any?(&:active)).to be(true) expect(get_runner_names(runner_matrix)).to eq(get_runner_names(runner_matrix, :macos?)) @@ -172,7 +195,7 @@ allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true) allow(Formula).to receive(:all).and_return([testball, testball_depender_intel].map(&:formula)) - runner_matrix = described_class.new([testball], [], dependent_matrix: true) + runner_matrix = described_class.new([testball], [], all_supported: false, dependent_matrix: true) expect(runner_matrix.runners.all?(&:active)).to be(false) expect(runner_matrix.runners.any?(&:active)).to be(true) expect(get_runner_names(runner_matrix)).to eq(get_runner_names(runner_matrix, :x86_64?)) @@ -184,7 +207,7 @@ allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true) allow(Formula).to receive(:all).and_return([testball, testball_depender_arm].map(&:formula)) - runner_matrix = described_class.new([testball], [], dependent_matrix: true) + runner_matrix = described_class.new([testball], [], all_supported: false, dependent_matrix: true) expect(runner_matrix.runners.all?(&:active)).to be(false) expect(runner_matrix.runners.any?(&:active)).to be(true) expect(get_runner_names(runner_matrix)).to eq(get_runner_names(runner_matrix, :arm64?)) @@ -197,7 +220,9 @@ context "when there are deleted formulae" do context "when it is a matrix for the `tests` job" do it "activates all runners" do - expect(described_class.new([], ["deleted"], dependent_matrix: false).runners.all?(&:active)) + expect(described_class.new([], ["deleted"], all_supported: false, dependent_matrix: false) + .runners + .all?(&:active)) .to be(true) end end @@ -205,7 +230,9 @@ context "when it is a matrix for the `test_deps` job" do context "when there are no testing formulae" do it "activates no runners" do - expect(described_class.new([], ["deleted"], dependent_matrix: true).runners.any?(&:active)) + expect(described_class.new([], ["deleted"], all_supported: false, dependent_matrix: true) + .runners + .any?(&:active)) .to be(false) end end @@ -213,7 +240,9 @@ context "when there are testing formulae with no dependents" do it "activates no runners" do testing_formulae = [testball] - runner_matrix = described_class.new(testing_formulae, ["deleted"], dependent_matrix: true) + runner_matrix = described_class.new(testing_formulae, ["deleted"], + all_supported: false, + dependent_matrix: true) allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true) allow(Formula).to receive(:all).and_return(testing_formulae.map(&:formula)) @@ -229,7 +258,9 @@ allow(Formula).to receive(:all).and_return([testball, testball_depender].map(&:formula)) testing_formulae = [testball] - expect(described_class.new(testing_formulae, ["deleted"], dependent_matrix: true).runners.all?(&:active)) + expect(described_class.new(testing_formulae, ["deleted"], all_supported: false, dependent_matrix: true) + .runners + .all?(&:active)) .to be(true) end end @@ -240,8 +271,7 @@ allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true) allow(Formula).to receive(:all).and_return([testball, testball_depender_linux].map(&:formula)) - testing_formulae = [testball] - matrix = described_class.new(testing_formulae, ["deleted"], dependent_matrix: true) + matrix = described_class.new([testball], ["deleted"], all_supported: false, dependent_matrix: true) expect(get_runner_names(matrix)).to eq(["Linux"]) end end @@ -251,8 +281,7 @@ allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true) allow(Formula).to receive(:all).and_return([testball, testball_depender_macos].map(&:formula)) - testing_formulae = [testball] - matrix = described_class.new(testing_formulae, ["deleted"], dependent_matrix: true) + matrix = described_class.new([testball], ["deleted"], all_supported: false, dependent_matrix: true) expect(get_runner_names(matrix)).to eq(get_runner_names(matrix, :macos?)) end end From 508fc2a0e397148c361c39ee0ca56c32a43f5637 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Fri, 17 Nov 2023 01:38:27 +0000 Subject: [PATCH 2/2] workflows/doctor: use brew determine-test-runners --- .github/workflows/doctor.yml | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/.github/workflows/doctor.yml b/.github/workflows/doctor.yml index 653b8b52de0f7..7a260e7cff58e 100644 --- a/.github/workflows/doctor.yml +++ b/.github/workflows/doctor.yml @@ -14,21 +14,34 @@ env: HOMEBREW_DEVELOPER: 1 HOMEBREW_NO_AUTO_UPDATE: 1 jobs: + determine-runners: + runs-on: ubuntu-22.04 + outputs: + runners: ${{ steps.determine-runners.outputs.runners }} + steps: + - name: Set up Homebrew + id: set-up-homebrew + uses: Homebrew/actions/setup-homebrew@master + with: + core: false + cask: false + test-bot: false + + - name: Determine runners to use for this job + id: determine-runners + env: + HOMEBREW_MACOS_TIMEOUT: 30 + run: brew determine-test-runners --all-supported + tests: + needs: determine-runners strategy: matrix: - include: - - runner: "13-arm64-${{ github.run_id }}" - - runner: "13-${{ github.run_id }}" - - runner: "12-arm64-${{ github.run_id }}" - - runner: "12-${{ github.run_id }}" - - runner: "11-arm64" - cleanup: true - - runner: "11-${{ github.run_id }}" + include: ${{ fromJson(needs.determine-runners.outputs.runners) }} fail-fast: false + name: ${{ matrix.name }} runs-on: ${{ matrix.runner }} - env: - PATH: "/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin" + timeout-minutes: ${{ matrix.timeout }} defaults: run: working-directory: /tmp