Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: detect all package changes #1006

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/test-bot.rb
Expand Up @@ -80,6 +80,8 @@ def test_bot_args
description: "Only run the formulae steps."
switch "--only-formulae-detect",
description: "Only run the formulae detection steps."
SMillerDev marked this conversation as resolved.
Show resolved Hide resolved
switch "--only-casks-detect",
description: "Only run the casks detection steps."
switch "--only-formulae-dependents",
description: "Only run the formulae dependents steps."
switch "--only-bottles-fetch",
Expand Down
19 changes: 16 additions & 3 deletions lib/test_runner.rb
Expand Up @@ -6,7 +6,7 @@
require_relative "test_formulae"
require_relative "tests/cleanup_after"
require_relative "tests/cleanup_before"
require_relative "tests/formulae_detect"
require_relative "tests/packages_detect"
require_relative "tests/formulae_dependents"
require_relative "tests/bottles_fetch"
require_relative "tests/formulae"
Expand Down Expand Up @@ -111,6 +111,7 @@ def no_only_args?(args)
args.only_tap_syntax? ||
args.only_formulae? ||
args.only_formulae_detect? ||
args.only_casks_detect? ||
args.only_formulae_dependents? ||
args.only_bottles_fetch? ||
args.only_cleanup_after?
Expand Down Expand Up @@ -139,12 +140,19 @@ def build_tests(argument, tap:, git:, output_paths:, skip_setup:,
no_formulae_flags = args.testing_formulae.nil? &&
args.added_formulae.nil? &&
args.deleted_formulae.nil?
if no_formulae_flags && (no_only_args || args.only_formulae? || args.only_formulae_detect?)
tests[:formulae_detect] = Tests::FormulaeDetect.new(argument, tap:,
if no_formulae_flags &&
(no_only_args || args.only_formulae? || args.only_formulae_detect?)
tests[:formulae_detect] = Tests::PackagesDetect.new(argument, tap:,
git:,
dry_run: args.dry_run?,
fail_fast: args.fail_fast?,
verbose: args.verbose?)
elsif no_only_args || args.only_casks_detect?
tests[:casks_detect] = Tests::PackagesDetect.new(argument, tap:,
git:,
dry_run: args.dry_run?,
fail_fast: args.fail_fast?,
verbose: args.verbose?)
end

if no_only_args || args.only_formulae?
Expand Down Expand Up @@ -199,6 +207,11 @@ def run_tests(tests, args:)
tests[:setup]&.run!(args:)
tests[:tap_syntax]&.run!(args:)

if (detect_test = tests[:casks_detect])
detect_test.run!(args:)
return
end

testing_formulae, added_formulae, deleted_formulae = if (detect_test = tests[:formulae_detect])
detect_test.run!(args:)

Expand Down
112 changes: 86 additions & 26 deletions lib/tests/formulae_detect.rb → lib/tests/packages_detect.rb
Expand Up @@ -2,20 +2,23 @@

module Homebrew
module Tests
class FormulaeDetect < Test
attr_reader :testing_formulae, :added_formulae, :deleted_formulae
class PackagesDetect < Test
attr_reader :testing_formulae, :added_formulae, :deleted_formulae,
:testing_casks, :added_casks, :deleted_casks

def initialize(argument, tap:, git:, dry_run:, fail_fast:, verbose:)
super(tap:, git:, dry_run:, fail_fast:, verbose:)

@argument = argument
@added_formulae = []
@added_casks = []
@deleted_formulae = []
@deleted_casks = []
@formulae_to_fetch = []
end

def run!(args:)
detect_formulae!(args:)
detect_packages!(args:)

return unless ENV["GITHUB_ACTIONS"]

Expand All @@ -24,13 +27,17 @@ def run!(args:)
f.puts "added_formulae=#{@added_formulae.join(",")}"
f.puts "deleted_formulae=#{@deleted_formulae.join(",")}"
f.puts "formulae_to_fetch=#{@formulae_to_fetch.join(",")}"

f.puts "testing_casks=#{@testing_casks.join(",")}"
f.puts "added_casks=#{@added_casks.join(",")}"
f.puts "deleted_casks=#{@deleted_casks.join(",")}"
end
end

private

def detect_formulae!(args:)
test_header(:FormulaeDetect, method: :detect_formulae!)
def detect_packages!(args:)
test_header(:PackagesDetect, method: :detect_packages!)

url = nil
origin_ref = "origin/master"
Expand All @@ -40,6 +47,7 @@ def detect_formulae!(args:)

if @argument == "HEAD"
@testing_formulae = []
@testing_casks = []
# Use GitHub Actions variables for pull request jobs.
if github_ref.present? && github_repository.present? &&
%r{refs/pull/(?<pr>\d+)/merge} =~ github_ref
Expand All @@ -52,6 +60,15 @@ def detect_formulae!(args:)
end

@testing_formulae = [canonical_formula_name]
@testing_casks = []
MikeMcQuaid marked this conversation as resolved.
Show resolved Hide resolved
elsif (canonical_cask_name = safe_cask_canonical_name(@argument, args:))
unless canonical_cask_name.include?("/")
ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1"
CoreTap.ensure_installed!
end

@testing_casks = [canonical_cask_name]
@testing_formulae = []
else
raise UsageError,
"#{@argument} is not detected from GitHub Actions or a formula name!"
Expand Down Expand Up @@ -101,7 +118,7 @@ def detect_formulae!(args:)
diff_start_sha1 = current_sha1 if diff_start_sha1.blank?
diff_end_sha1 = current_sha1 if diff_end_sha1.blank?

diff_start_sha1 = diff_end_sha1 if @testing_formulae.present?
diff_start_sha1 = diff_end_sha1 if @testing_formulae.present? || @testing_casks.present?

if tap
tap_origin_ref_revision_args =
Expand All @@ -127,34 +144,43 @@ def detect_formulae!(args:)
EOS

modified_formulae = []
modified_casks = []

if tap && diff_start_sha1 != diff_end_sha1
formula_path = tap.formula_dir.to_s
@added_formulae +=
diff_formulae(diff_start_sha1, diff_end_sha1, formula_path, "A")
modified_formulae +=
diff_formulae(diff_start_sha1, diff_end_sha1, formula_path, "M")
@deleted_formulae +=
diff_formulae(diff_start_sha1, diff_end_sha1, formula_path, "D")
@added_formulae += diff_packages(diff_start_sha1, diff_end_sha1, formula_path, "A")
modified_formulae += diff_packages(diff_start_sha1, diff_end_sha1, formula_path, "M")
@deleted_formulae += diff_packages(diff_start_sha1, diff_end_sha1, formula_path, "D")

cask_path = tap.cask_dir.to_s
@added_casks += diff_packages(diff_start_sha1, diff_end_sha1, cask_path, "A")
modified_casks += diff_packages(diff_start_sha1, diff_end_sha1, cask_path, "M")
@deleted_casks += diff_packages(diff_start_sha1, diff_end_sha1, cask_path, "D")
end

# If a formula is both added and deleted: it's actually modified.
# If a package is both added and deleted: it's actually modified.
added_and_deleted_formulae = @added_formulae & @deleted_formulae
@added_formulae -= added_and_deleted_formulae
@deleted_formulae -= added_and_deleted_formulae
modified_formulae += added_and_deleted_formulae

added_and_deleted_casks = @added_casks & @deleted_casks
@added_casks -= added_and_deleted_casks
@deleted_casks -= added_and_deleted_casks
modified_casks += added_and_deleted_casks

if args.test_default_formula?
# Build the default test formula.
modified_formulae << "homebrew/test-bot/testbottest"
end

@testing_formulae += @added_formulae + modified_formulae
@testing_casks += @added_casks + modified_casks

# TODO: Remove `GITHUB_EVENT_NAME` check when formulae detection
# is fixed for branch jobs.
if @testing_formulae.blank? &&
@deleted_formulae.blank? &&
if @testing_casks.blank? &&
@deleted_casks.blank? &&
diff_start_sha1 == diff_end_sha1 &&
(ENV["GITHUB_EVENT_NAME"] != "push")
raise UsageError, "Did not find any formulae or commits to test!"
Expand All @@ -166,6 +192,11 @@ def detect_formulae!(args:)
modified_formulae.uniq!
@deleted_formulae.uniq!

@testing_casks.uniq!
@added_casks.uniq!
modified_casks.uniq!
@deleted_casks.uniq!

# We only need to do a fetch test on formulae that have had a change in the pkg version or bottle block.
# These fetch tests only happen in merge queues.
@formulae_to_fetch = if diff_start_sha1 == diff_end_sha1 || ENV["GITHUB_EVENT_NAME"] != "merge_group"
Expand All @@ -186,14 +217,24 @@ def detect_formulae!(args:)
end
end

puts <<-EOS
if args.only_casks_detect?
puts <<-EOS

testing_formulae #{@testing_formulae.join(" ").presence || "(none)"}
added_formulae #{@added_formulae.join(" ").presence || "(none)"}
modified_formulae #{modified_formulae.join(" ").presence || "(none)"}
deleted_formulae #{@deleted_formulae.join(" ").presence || "(none)"}
formulae_to_fetch #{@formulae_to_fetch.join(" ").presence || "(none)"}
EOS
testing_casks #{@testing_casks.join(" ").presence || "(none)"}
added_casks #{@added_casks.join(" ").presence || "(none)"}
modified_casks #{modified_casks.join(" ").presence || "(none)"}
deleted_casks #{@deleted_casks.join(" ").presence || "(none)"}
EOS
else
puts <<-EOS

testing_formulae #{@testing_formulae.join(" ").presence || "(none)"}
added_formulae #{@added_formulae.join(" ").presence || "(none)"}
modified_formulae #{modified_formulae.join(" ").presence || "(none)"}
deleted_formulae #{@deleted_formulae.join(" ").presence || "(none)"}
formulae_to_fetch #{@formulae_to_fetch.join(" ").presence || "(none)"}
EOS
end
end

def safe_formula_canonical_name(formula_name, args:)
Expand All @@ -213,6 +254,22 @@ def safe_formula_canonical_name(formula_name, args:)
puts e.backtrace if args.debug?
end

def safe_cask_canonical_name(cask_name, args:)
Homebrew.with_no_api_env do
CaskLoader.load(cask_name).full_name
end
rescue TapCaskUnavailableError => e
raise if e.tap.installed?

test "brew", "tap", e.tap.name
retry unless steps.last.failed?
onoe e
puts e.backtrace if args.debug?
rescue CaskUnavailableError, TapCaskAmbiguityError => e
onoe e
puts e.backtrace if args.debug?
end

def rev_parse(ref)
Utils.popen_read(git, "-C", repository, "rev-parse", "--verify", ref).strip
end
Expand All @@ -221,7 +278,7 @@ def current_sha1
rev_parse("HEAD")
end

def diff_formulae(start_revision, end_revision, path, filter)
def diff_packages(start_revision, end_revision, path, filter)
return unless tap

Utils.safe_popen_read(
Expand All @@ -230,10 +287,13 @@ def diff_formulae(start_revision, end_revision, path, filter)
start_revision, end_revision, "--", path
).lines.filter_map do |line|
file = Pathname.new line.chomp
next unless tap.formula_file?(file)

tap.formula_file_to_name(file)
end
if tap.formula_file?(file)
tap.formula_file_to_name(file)
elsif tap.cask_file?(file)
file.basename(".rb").to_s
end
end.compact
end
end
end
Expand Down