From 3588f1b8c568adf995e9c6719816256cb0d85e1d Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Mon, 26 Aug 2024 08:10:31 +0800 Subject: [PATCH] github_runner_matrix: filter incompatible testing formulae `brew test-bot` can occasionally spend a long time doing nothing when testing dependents.[^1] This is because it takes a long time to work out that there is nothing to do. To fix this, let's adjust `brew determine-test-runners` to pass on only the list of formulae for which there is work to be done so that `brew test-bot` doesn't need to waste time working this out. [^1]: For example, it spends about 15 minutes doing nothing at https://github.com/Homebrew/homebrew-core/actions/runs/10500178069/job/29133091332?pr=180185 --- Library/Homebrew/github_runner_matrix.rb | 92 ++++++++++++++---------- Library/Homebrew/linux_runner_spec.rb | 15 ++-- Library/Homebrew/macos_runner_spec.rb | 12 +++- 3 files changed, 74 insertions(+), 45 deletions(-) diff --git a/Library/Homebrew/github_runner_matrix.rb b/Library/Homebrew/github_runner_matrix.rb index cf1715f2d7..ea99a7cbc5 100644 --- a/Library/Homebrew/github_runner_matrix.rb +++ b/Library/Homebrew/github_runner_matrix.rb @@ -10,17 +10,26 @@ class GitHubRunnerMatrix RunnerSpec = T.type_alias { T.any(LinuxRunnerSpec, MacOSRunnerSpec) } private_constant :RunnerSpec - MacOSRunnerSpecHash = T.type_alias { { name: String, runner: String, timeout: Integer, cleanup: T::Boolean } } + MacOSRunnerSpecHash = T.type_alias do + { + name: String, + runner: String, + timeout: Integer, + cleanup: T::Boolean, + testing_formulae: String, + } + end private_constant :MacOSRunnerSpecHash LinuxRunnerSpecHash = T.type_alias do { - name: String, - runner: String, - container: T::Hash[Symbol, String], - workdir: String, - timeout: Integer, - cleanup: T::Boolean, + name: String, + runner: String, + container: T::Hash[Symbol, String], + workdir: String, + timeout: Integer, + cleanup: T::Boolean, + testing_formulae: String, } end private_constant :LinuxRunnerSpecHash @@ -49,6 +58,8 @@ class GitHubRunnerMatrix @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) + @compatible_testing_formulae = T.let({}, T::Hash[GitHubRunner, T::Array[TestRunnerFormula]]) + @formulae_with_untested_dependents = T.let({}, T::Hash[GitHubRunner, T::Array[TestRunnerFormula]]) @runners = T.let([], T::Array[GitHubRunner]) generate_runners! @@ -102,6 +113,7 @@ class GitHubRunnerMatrix raise "Unexpected arch: #{arch}" if VALID_ARCHES.exclude?(arch) runner = GitHubRunner.new(platform:, arch:, spec:, macos_version:) + runner.spec.testing_formulae += testable_formulae(runner) runner.active = active_runner?(runner) runner.freeze end @@ -185,56 +197,60 @@ class GitHubRunnerMatrix @runners.freeze end + sig { params(runner: GitHubRunner).returns(T::Array[String]) } + def testable_formulae(runner) + formulae = if @dependent_matrix + formulae_with_untested_dependents(runner) + else + compatible_testing_formulae(runner) + end + + formulae.map(&:name) + end + sig { params(runner: GitHubRunner).returns(T::Boolean) } def active_runner?(runner) - if @dependent_matrix - formulae_have_untested_dependents?(runner) - elsif !@all_supported && @deleted_formulae.empty? - compatible_formulae = @testing_formulae.dup + return true if @all_supported || @deleted_formulae.present? + testable_formulae(runner).present? + end + + sig { params(runner: GitHubRunner).returns(T::Array[TestRunnerFormula]) } + def compatible_testing_formulae(runner) + @compatible_testing_formulae[runner] ||= begin platform = runner.platform arch = runner.arch macos_version = runner.macos_version - compatible_formulae.select! do |formula| + @testing_formulae.select do |formula| next false if macos_version && !formula.compatible_with?(macos_version) formula.public_send(:"#{platform}_compatible?") && formula.public_send(:"#{arch}_compatible?") end - - compatible_formulae.present? - else - true end end - sig { params(runner: GitHubRunner).returns(T::Boolean) } - def formulae_have_untested_dependents?(runner) - platform = runner.platform - arch = runner.arch - macos_version = runner.macos_version + sig { params(runner: GitHubRunner).returns(T::Array[TestRunnerFormula]) } + def formulae_with_untested_dependents(runner) + @formulae_with_untested_dependents[runner] ||= begin + 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 - # dependents don't need to be tested if these requirements are not satisfied. - next false unless formula.public_send(:"#{platform}_compatible?") - next false unless formula.public_send(:"#{arch}_compatible?") - next false if macos_version.present? && !formula.compatible_with?(macos_version) + compatible_testing_formulae(runner).select do |formula| + compatible_dependents = formula.dependents(platform:, arch:, macos_version: macos_version&.to_sym) + .select do |dependent_f| + next false if macos_version && !dependent_f.compatible_with?(macos_version) - compatible_dependents = formula.dependents(platform:, arch:, macos_version: macos_version&.to_sym) - .dup + dependent_f.public_send(:"#{platform}_compatible?") && + dependent_f.public_send(:"#{arch}_compatible?") + end - compatible_dependents.select! do |dependent_f| - next false if macos_version && !dependent_f.compatible_with?(macos_version) - - dependent_f.public_send(:"#{platform}_compatible?") && - dependent_f.public_send(:"#{arch}_compatible?") + # These arrays will generally have been generated by different Formulary caches, + # so we can only compare them by name and not directly. + (compatible_dependents.map(&:name) - @testing_formulae.map(&:name)).present? end - - # These arrays will generally have been generated by different Formulary caches, - # so we can only compare them by name and not directly. - (compatible_dependents.map(&:name) - @testing_formulae.map(&:name)).present? end end end diff --git a/Library/Homebrew/linux_runner_spec.rb b/Library/Homebrew/linux_runner_spec.rb index 43244819c0..be097267fc 100644 --- a/Library/Homebrew/linux_runner_spec.rb +++ b/Library/Homebrew/linux_runner_spec.rb @@ -8,15 +8,17 @@ class LinuxRunnerSpec < T::Struct const :workdir, String const :timeout, Integer const :cleanup, T::Boolean + prop :testing_formulae, T::Array[String], default: [] sig { returns({ - name: String, - runner: String, - container: T::Hash[Symbol, String], - workdir: String, - timeout: Integer, - cleanup: T::Boolean, + name: String, + runner: String, + container: T::Hash[Symbol, String], + workdir: String, + timeout: Integer, + cleanup: T::Boolean, + testing_formulae: String, }) } def to_h @@ -27,6 +29,7 @@ class LinuxRunnerSpec < T::Struct workdir:, timeout:, cleanup:, + testing_formulae: testing_formulae.join(","), } end end diff --git a/Library/Homebrew/macos_runner_spec.rb b/Library/Homebrew/macos_runner_spec.rb index d8f1f0f47f..40cd561e62 100644 --- a/Library/Homebrew/macos_runner_spec.rb +++ b/Library/Homebrew/macos_runner_spec.rb @@ -6,14 +6,24 @@ class MacOSRunnerSpec < T::Struct const :runner, String const :timeout, Integer const :cleanup, T::Boolean + prop :testing_formulae, T::Array[String], default: [] - sig { returns({ name: String, runner: String, timeout: Integer, cleanup: T::Boolean }) } + sig { + returns({ + name: String, + runner: String, + timeout: Integer, + cleanup: T::Boolean, + testing_formulae: String, + }) + } def to_h { name:, runner:, timeout:, cleanup:, + testing_formulae: testing_formulae.join(","), } end end