From 87373ff12abfea1145302011581bd22b0d5edaad Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Thu, 6 Apr 2023 02:13:15 +0800 Subject: [PATCH] Move more code out of `dev-cmd` --- .../dev-cmd/determine-test-runners.rb | 70 +------ Library/Homebrew/github_runner_matrix.rb | 193 +++++++++++++----- .../dev-cmd/determine-test-runners_spec.rb | 1 - Library/Homebrew/test_runner_formula.rb | 1 + 4 files changed, 153 insertions(+), 112 deletions(-) diff --git a/Library/Homebrew/dev-cmd/determine-test-runners.rb b/Library/Homebrew/dev-cmd/determine-test-runners.rb index 92b2ebc98d..40eec0e817 100755 --- a/Library/Homebrew/dev-cmd/determine-test-runners.rb +++ b/Library/Homebrew/dev-cmd/determine-test-runners.rb @@ -28,22 +28,6 @@ module Homebrew end end - sig { - params( - version: String, - arch: Symbol, - ephemeral: T::Boolean, - ephemeral_suffix: T.nilable(String), - ).returns(T::Hash[Symbol, T.any(String, T::Boolean)]) - } - def self.runner_spec(version, arch:, ephemeral:, ephemeral_suffix: nil) - case arch - when :arm64 then { runner: "#{version}-arm64#{ephemeral_suffix}", clean: !ephemeral } - when :x86_64 then { runner: "#{version}#{ephemeral_suffix}", clean: !ephemeral } - else raise "Unexpected arch: #{arch}" - end - end - sig { void } def self.determine_test_runners args = determine_test_runners_args.parse @@ -52,65 +36,21 @@ module Homebrew odie "`--dependents` requires `--eval-all` or `HOMEBREW_EVAL_ALL`!" if args.dependents? && !eval_all - Formulary.enable_factory_cache! - testing_formulae = args.named.first.split(",") testing_formulae.map! { |name| TestRunnerFormula.new(Formulary.factory(name), eval_all: eval_all) } .freeze - deleted_formulae = args.named.second&.split(",") + deleted_formulae = args.named.second&.split(",").freeze - linux_runner = ENV.fetch("HOMEBREW_LINUX_RUNNER") { raise "HOMEBREW_LINUX_RUNNER is not defined" } - linux_cleanup = ENV.fetch("HOMEBREW_LINUX_CLEANUP") { raise "HOMEBREW_LINUX_CLEANUP is not defined" } - github_run_id = ENV.fetch("GITHUB_RUN_ID") { raise "GITHUB_RUN_ID is not defined" } - github_run_attempt = ENV.fetch("GITHUB_RUN_ATTEMPT") { raise "GITHUB_RUN_ATTEMPT is not defined" } - github_output = ENV.fetch("GITHUB_OUTPUT") { raise "GITHUB_OUTPUT is not defined" } - - linux_runner_spec = { - runner: linux_runner, - container: { - image: "ghcr.io/homebrew/ubuntu22.04:master", - options: "--user=linuxbrew -e GITHUB_ACTIONS_HOMEBREW_SELF_HOSTED", - }, - workdir: "/github/home", - timeout: 4320, - cleanup: linux_cleanup == "true", - } - ephemeral_suffix = "-#{github_run_id}-#{github_run_attempt}" - - available_runners = [] - available_runners << { platform: :linux, arch: :x86_64, runner_spec: linux_runner_spec, macos_version: nil } - - MacOSVersions::SYMBOLS.each_value do |version| - macos_version = OS::Mac::Version.new(version) - next if macos_version.outdated_release? || macos_version.prerelease? - - spec = runner_spec(version, arch: :x86_64, ephemeral: true, ephemeral_suffix: ephemeral_suffix) - available_runners << { platform: :macos, arch: :x86_64, runner_spec: spec, macos_version: macos_version } - - # Use bare metal runner when testing dependents on ARM64 Monterey. - if (macos_version >= :ventura && args.dependents?) || macos_version >= :monterey - spec = runner_spec(version, arch: :arm64, ephemeral: true, ephemeral_suffix: ephemeral_suffix) - available_runners << { platform: :macos, arch: :arm64, runner_spec: spec, macos_version: macos_version } - elsif macos_version >= :big_sur - spec = runner_spec(version, arch: :arm64, ephemeral: false) - available_runners << { platform: :macos, arch: :arm64, runner_spec: spec, macos_version: macos_version } - end - end - - runner_matrix = GitHubRunnerMatrix.new( - available_runners, - testing_formulae, - deleted_formulae, - dependent_matrix: args.dependents?, - ) - runners = runner_matrix.active_runners + runner_matrix = GitHubRunnerMatrix.new(testing_formulae, deleted_formulae, dependent_matrix: args.dependents?) + runners = runner_matrix.active_runner_specs_hash if !args.dependents? && runners.blank? # If there are no tests to run, add a runner that is meant to do nothing # to support making the `tests` job a required status check. - runners << { runner: "ubuntu-latest", no_op: true } + runners << { name: "macOS 13-arm64", runner: "ubuntu-latest", no_op: true } end + github_output = ENV.fetch("GITHUB_OUTPUT") File.open(github_output, "a") do |f| f.puts("runners=#{runners.to_json}") f.puts("runners_present=#{runners.present?}") diff --git a/Library/Homebrew/github_runner_matrix.rb b/Library/Homebrew/github_runner_matrix.rb index f32d238628..7f408b270f 100644 --- a/Library/Homebrew/github_runner_matrix.rb +++ b/Library/Homebrew/github_runner_matrix.rb @@ -3,72 +3,169 @@ require "test_runner_formula" +class LinuxRunnerSpec < T::Struct + extend T::Sig + + const :name, String + const :runner, String + const :container, T::Hash[Symbol, String] + const :workdir, String + const :timeout, Integer + const :cleanup, T::Boolean + + sig { + returns({ + name: String, + runner: String, + container: T::Hash[Symbol, String], + workdir: String, + timeout: Integer, + cleanup: T::Boolean, + }) + } + def to_h + { + name: name, + runner: runner, + container: container, + workdir: workdir, + timeout: timeout, + cleanup: cleanup, + } + end +end + +class MacOSRunnerSpec < T::Struct + extend T::Sig + + const :name, String + const :runner, String + const :cleanup, T::Boolean + + sig { returns({ name: String, runner: String, cleanup: T::Boolean }) } + def to_h + { + name: name, + runner: runner, + cleanup: cleanup, + } + end +end + +class GitHubRunner < T::Struct + const :platform, Symbol + const :arch, Symbol + const :spec, T.any(LinuxRunnerSpec, MacOSRunnerSpec) + const :macos_version, T.nilable(OS::Mac::Version) +end + class GitHubRunnerMatrix extend T::Sig - # FIXME: sig { returns(T::Array[RunnerSpec]) } - sig { returns(T::Array[RunnerHashValue]) } - attr_reader :active_runners - # FIXME: Enable cop again when https://github.com/sorbet/sorbet/issues/3532 is fixed. # rubocop:disable Style/MutableConstant - RunnerSpec = T.type_alias do - T.any( - T::Hash[Symbol, T.any(String, T::Hash[Symbol, String], Integer, T::Boolean)], # Linux - T::Hash[Symbol, T.any(String, T::Boolean)], # macOS - ) - end + MaybeStringArray = T.type_alias { T.nilable(T::Array[String]) } + private_constant :MaybeStringArray + + RunnerSpec = T.type_alias { T.any(LinuxRunnerSpec, MacOSRunnerSpec) } private_constant :RunnerSpec - RunnerHashValue = T.type_alias { T.any(Symbol, RunnerSpec, T.nilable(OS::Mac::Version)) } - private_constant :RunnerHashValue # rubocop:enable Style/MutableConstant + sig { returns(T::Array[GitHubRunner]) } + attr_reader :available_runners + sig { params( - available_runners: T::Array[T::Hash[Symbol, RunnerHashValue]], - testing_formulae: T::Array[TestRunnerFormula], - deleted_formulae: T.nilable(T::Array[String]), - dependent_matrix: T::Boolean, + testing_formulae: T::Array[TestRunnerFormula], + deleted_formulae: MaybeStringArray, + dependent_matrix: T::Boolean, ).void } - def initialize(available_runners, testing_formulae, deleted_formulae, dependent_matrix:) - @available_runners = T.let(available_runners, T::Array[T::Hash[Symbol, RunnerHashValue]]) + def initialize(testing_formulae, deleted_formulae, dependent_matrix:) @testing_formulae = T.let(testing_formulae, T::Array[TestRunnerFormula]) - @deleted_formulae = T.let(deleted_formulae, T.nilable(T::Array[String])) + @deleted_formulae = T.let(deleted_formulae, MaybeStringArray) @dependent_matrix = T.let(dependent_matrix, T::Boolean) - # FIXME: Should have type `RunnerSpec`, but Sorbet can't infer that that's correct. - @active_runners = T.let([], T::Array[RunnerHashValue]) - generate_runners! + @available_runners = T.let([], T::Array[GitHubRunner]) + generate_available_runners! + + @active_runners = T.let( + @available_runners.select { |runner| active_runner?(runner) }, + T::Array[GitHubRunner], + ) freeze end + sig { returns(T::Array[T::Hash[Symbol, T.untyped]]) } + def active_runner_specs_hash + @active_runners.map(&:spec) + .map(&:to_h) + end + + sig { returns(LinuxRunnerSpec) } + def linux_runner_spec + linux_runner = ENV.fetch("HOMEBREW_LINUX_RUNNER") + linux_cleanup = ENV.fetch("HOMEBREW_LINUX_CLEANUP") + + LinuxRunnerSpec.new( + name: "Linux", + runner: linux_runner, + container: { + image: "ghcr.io/homebrew/ubuntu22.04:master", + options: "--user=linuxbrew -e GITHUB_ACTIONS_HOMEBREW_SELF_HOSTED", + }, + workdir: "/github/home", + timeout: 4320, + cleanup: linux_cleanup == "true", + ) + end + sig { void } - def generate_runners! - @available_runners.each do |runner| - @active_runners << runner.fetch(:runner_spec) if add_runner?(runner) + def generate_available_runners! + @available_runners << GitHubRunner.new(platform: :linux, arch: :x86_64, spec: linux_runner_spec) + + github_run_id = ENV.fetch("GITHUB_RUN_ID") + github_run_attempt = ENV.fetch("GITHUB_RUN_ATTEMPT") + ephemeral_suffix = "-#{github_run_id}-#{github_run_attempt}" + + MacOSVersions::SYMBOLS.each_value do |version| + macos_version = OS::Mac::Version.new(version) + next if macos_version.outdated_release? || macos_version.prerelease? + + spec = MacOSRunnerSpec.new( + name: "macOS #{version}-x86_64", + runner: "#{version}#{ephemeral_suffix}", + cleanup: false, + ) + @available_runners << GitHubRunner.new( + platform: :macos, + arch: :x86_64, + spec: spec, + macos_version: macos_version, + ) + + next unless macos_version >= :big_sur + + # Use bare metal runner when testing dependents on ARM64 Monterey. + runner, cleanup = if (macos_version >= :ventura && @dependent_matrix) || macos_version >= :monterey + ["#{version}-arm64#{ephemeral_suffix}", false] + else + ["#{version}-arm64", true] + end + + spec = MacOSRunnerSpec.new(name: "macOS #{version}-arm64", runner: runner, cleanup: cleanup) + @available_runners << GitHubRunner.new( + platform: :macos, + arch: :arm64, + spec: spec, + macos_version: macos_version, + ) end end - sig { params(runner: T::Hash[Symbol, RunnerHashValue]).returns([Symbol, Symbol, T.nilable(OS::Mac::Version)]) } - def unpack_runner(runner) - platform = runner.fetch(:platform) - raise "Unexpected platform: #{platform}" if !platform.is_a?(Symbol) || [:macos, :linux].exclude?(platform) - - arch = runner.fetch(:arch) - raise "Unexpected arch: #{arch}" if !arch.is_a?(Symbol) || [:arm64, :x86_64].exclude?(arch) - - macos_version = runner.fetch(:macos_version) - if !macos_version.nil? && !macos_version.is_a?(OS::Mac::Version) - raise "Unexpected macos_version: #{macos_version}" - end - - [platform, arch, macos_version] - end - - sig { params(runner: T::Hash[Symbol, RunnerHashValue]).returns(T::Boolean) } - def add_runner?(runner) + sig { params(runner: GitHubRunner).returns(T::Boolean) } + def active_runner?(runner) if @dependent_matrix formulae_have_untested_dependents?(runner) else @@ -76,7 +173,9 @@ class GitHubRunnerMatrix compatible_formulae = @testing_formulae.dup - platform, arch, macos_version = unpack_runner(runner) + platform = runner.platform + arch = runner.arch + macos_version = runner.macos_version compatible_formulae.select! { |formula| formula.send(:"#{platform}_compatible?") } compatible_formulae.select! { |formula| formula.send(:"#{arch}_compatible?") } @@ -86,9 +185,11 @@ class GitHubRunnerMatrix end end - sig { params(runner: T::Hash[Symbol, RunnerHashValue]).returns(T::Boolean) } + sig { params(runner: GitHubRunner).returns(T::Boolean) } def formulae_have_untested_dependents?(runner) - platform, arch, macos_version = unpack_runner(runner) + platform = runner.platform + arch = runner.arch + macos_version = runner.macos_version @testing_formulae.any? do |formula| # If the formula has a platform/arch/macOS version requirement, then its diff --git a/Library/Homebrew/test/dev-cmd/determine-test-runners_spec.rb b/Library/Homebrew/test/dev-cmd/determine-test-runners_spec.rb index f6da916c26..3a0e2ddf30 100644 --- a/Library/Homebrew/test/dev-cmd/determine-test-runners_spec.rb +++ b/Library/Homebrew/test/dev-cmd/determine-test-runners_spec.rb @@ -44,7 +44,6 @@ describe "brew determine-test-runners" do expect { brew "determine-test-runners", "testball", runner_env_dup } .to not_to_output.to_stdout - .and output("Error: #{k} is not defined\n").to_stderr .and be_a_failure end end diff --git a/Library/Homebrew/test_runner_formula.rb b/Library/Homebrew/test_runner_formula.rb index 215af2eee6..f623b4f13d 100644 --- a/Library/Homebrew/test_runner_formula.rb +++ b/Library/Homebrew/test_runner_formula.rb @@ -17,6 +17,7 @@ class TestRunnerFormula sig { params(formula: Formula, eval_all: T::Boolean).void } def initialize(formula, eval_all: Homebrew::EnvConfig.eval_all?) + Formulary.enable_factory_cache! @formula = T.let(formula, Formula) @name = T.let(formula.name, String) @dependent_hash = T.let({}, T::Hash[Symbol, T::Array[TestRunnerFormula]])