From da3b10aa51b1c68c78715553e12366e6dc79b366 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Mon, 26 Oct 2026 13:17:04 -0600 Subject: [PATCH 1/2] Check regression coverage for pull requests --- .github/workflows/regression-check.yml | 49 +++ docs/CHANGELOG.md | 4 + docs/CONTRIBUTING.md | 33 ++ script/regression_check.rb | 439 +++++++++++++++++++++++++ script/regression_check/runner.rb | 70 ++++ script/regression_check_history.rb | 116 +++++++ test/script/regression_check_test.rb | 388 ++++++++++++++++++++++ 7 files changed, 1099 insertions(+) create mode 100644 .github/workflows/regression-check.yml create mode 100644 script/regression_check.rb create mode 100644 script/regression_check/runner.rb create mode 100644 script/regression_check_history.rb create mode 100644 test/script/regression_check_test.rb diff --git a/.github/workflows/regression-check.yml b/.github/workflows/regression-check.yml new file mode 100644 index 000000000..698990cf9 --- /dev/null +++ b/.github/workflows/regression-check.yml @@ -0,0 +1,49 @@ +name: Regression coverage + +permissions: + contents: read + +on: + pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] + +jobs: + regression-check: + name: Added test regression check + runs-on: ubuntu-latest + timeout-minutes: 20 + env: + BUNDLE_GEMFILE: gemfiles/rails_8.1.gemfile + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 + with: + fetch-depth: 0 + persist-credentials: false + - name: Setup Ruby + uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1 + with: + ruby-version: 4.0 + bundler-cache: true + - name: Compare added and modified tests without the implementation + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + REQUIRE_REGRESSION: ${{ contains(github.event.pull_request.labels.*.name, 'regression-test') }} + shell: bash + run: | + args=(--base "$BASE_SHA" --output tmp/regression-check) + if [ "$REQUIRE_REGRESSION" = true ]; then + args+=(--strict) + fi + ruby script/regression_check.rb "${args[@]}" + - name: Summarize regression results + if: always() + run: | + if [ -f tmp/regression-check/summary.md ]; then + cat tmp/regression-check/summary.md >> "$GITHUB_STEP_SUMMARY" + fi + - name: Upload regression results + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5 + if: always() + with: + name: regression-check + path: tmp/regression-check diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 8275bab74..35e946d84 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,10 @@ nav_order: 6 ## main +* Add an informational CI check that runs added and modified tests without a pull request's implementation changes to identify regression coverage. + + *Joel Hawksley* + * Fix `Couldn't find template for digesting` errors when computing cache digests for templates that render components. Action View inferred a partial from `render SomeComponent.new`, so partials inferred from component renders are now discarded. Component renders are found with Prism rather than a regular expression, adding a dependency on the `prism` gem. *Joel Hawksley* diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index dff09681d..2d698ca2f 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -102,6 +102,39 @@ bundle exec ruby performance/measure.rb tmp/performance/head.json bundle exec ruby performance/compare.rb tmp/performance/base.json tmp/performance/head.json ``` +### Regression coverage + +The **Added test regression check** CI job runs added and modified Minitest tests on Ruby 4.0 and Rails 8.1. It repeats the same tests with packaged `app/` and `lib/` implementation files restored to the pull request's merge base. Both runs retain the pull request's dependencies, tests, and fixtures, including `app/views/test_mailer/`. + +Each test has its own result: + +| Result | Meaning | +| --- | --- | +| `demonstrated` | Passes on the pull request. An assertion fails without the implementation. | +| `not_demonstrated` | Also passes without the implementation. Review whether it exercises the change. | +| `head_failed` | Fails or errors on the pull request. | +| `inconclusive` | Errors, skips, times out, makes no assertions, or doesn't run. Inspect the logs. | + +Results are informational by default. The `regression-test` label requires every selected test to demonstrate a regression, with no warnings or unsupported cases. An assertion failure demonstrates a behavioral difference, not necessarily a bug. New APIs can produce legitimate pre-fix exceptions. Those remain inconclusive rather than counting arbitrary errors as evidence. + +The job selects individual `def test_*` methods and literal Rails `test "..."` blocks using Prism. RSpec and dynamic definitions are reported as unsupported. Changes only to helpers, shared setup, or fixtures don't automatically select existing tests. Dependency changes are reported but not reverted. The Actions summary and `regression-check` artifact contain results, exceptions, and process logs. + +To run locally against committed changes: + +```command +ruby script/regression_check.rb --base origin/main +``` + +Use `--strict` to require demonstrated regressions and `--install` to install missing dependencies in the temporary checkout. The working tree is never modified. + +To audit the latest 100 pull requests, including open and closed pull requests: + +```command +ruby script/regression_check_history.rb --limit 100 +``` + +The audit uses `gh`, freezes the PR list, fetches their revisions, and writes a per-test report and aggregate summary under `tmp/regression-history`. Repeating the command resumes comparisons made with the same checker from that directory. Historical dependency or runtime incompatibilities aren't counted as coverage results. + ## Documentation ### Previewing changes locally diff --git a/script/regression_check.rb b/script/regression_check.rb new file mode 100644 index 000000000..d3b62d0ca --- /dev/null +++ b/script/regression_check.rb @@ -0,0 +1,439 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require "fileutils" +require "bundler" +require "digest" +require "json" +require "open3" +require "optparse" +require "pathname" +require "prism" +require "rbconfig" +require "tmpdir" + +module RegressionCheck + ROOT = File.expand_path("..", __dir__) + RUNNER = File.expand_path("regression_check/runner.rb", __dir__) + CHECKER_DIGEST = Digest::SHA256.hexdigest(File.read(__FILE__) + File.read(RUNNER)) + + class Error < StandardError; end + + class TestDefinitions < Prism::Visitor + attr_reader :tests, :unsupported + + def initialize(source) + @tests = {} + @unsupported = [] + @scope = [] + @visibility = [:public] + result = Prism.parse(source) + raise Error, result.errors.map(&:message).join("; ") unless result.success? + + result.value.accept(self) + @tests.reject! { |_, test| test.delete("visibility") != :public } + end + + def visit_class_node(node) + within_scope(node.constant_path.slice) { visit(node.body) } + end + + def visit_module_node(node) + within_scope(node.constant_path.slice) { visit(node.body) } + end + + def visit_def_node(node) + add(node.name.to_s, node) if node.name.to_s.start_with?("test_") && !node.receiver + end + + def visit_call_node(node) + if [:public, :private, :protected].include?(node.name) && !node.receiver + if node.arguments + node.arguments.arguments.each do |argument| + if argument.is_a?(Prism::DefNode) + previous = @visibility[-1] + @visibility[-1] = node.name + visit(argument) + @visibility[-1] = previous + elsif argument.is_a?(Prism::SymbolNode) + test = @tests["#{@scope.join("::")}##{argument.unescaped}"] + test["visibility"] = node.name if test + end + end + else + @visibility[-1] = node.name + end + return + end + if node.name == :test && node.block + argument = node.arguments&.arguments&.first + if argument.is_a?(Prism::StringNode) && !node.receiver + add("test_#{argument.unescaped.gsub(/\s+/, "_")}", node) + else + @unsupported << "dynamic test definition at line #{node.location.start_line}" + end + elsif [:it, :specify, :define_method].include?(node.name) + @unsupported << "#{node.name} definition at line #{node.location.start_line}" + end + super + end + + private + + def within_scope(name) + @scope.push(name) + @visibility.push(:public) + yield + ensure + @scope.pop + @visibility.pop + end + + def add(name, node) + key = "#{@scope.join("::")}##{name}" + if @tests.key?(key) + @unsupported << "duplicate test identity #{key}" + end + @tests[key] = { + "name" => name, + "line" => node.location.start_line, + "end_line" => node.location.end_line, + "visibility" => @visibility.last, + "source" => node.slice + } + end + end + + class Check + def initialize(base:, output:, head: "HEAD", timeout: 90, install: false, root: ROOT) + @root = root + @base = git("rev-parse", "--verify", "#{base}^{commit}").strip + @head = git("rev-parse", "--verify", "#{head}^{commit}").strip + @base = git("merge-base", @base, @head).strip + @output = File.expand_path(output) + @timeout = timeout + @install = install + bundle_path = Bundler.settings[:path] + @bundle_path = File.expand_path(bundle_path, Bundler.root) if bundle_path + @report = { + "checker_digest" => CHECKER_DIGEST, + "base" => @base, "head" => @head, "tests" => [], "notes" => [], "files" => [], "changed_files" => [] + } + end + + def run + FileUtils.mkdir_p(@output) + FileUtils.rm_f([File.join(@output, "results.json"), File.join(@output, "summary.md")]) + selected = select_tests + @report["notes"] << "No added or modified Minitest tests found." if selected.empty? + unless selected.empty? + FileUtils.mkdir_p(File.join(@root, "tmp")) + Dir.mktmpdir("regression-check-", File.join(@root, "tmp")) do |scratch| + head = File.join(scratch, "head") + base = File.join(scratch, "base") + snapshot(@head, head) + install_dependencies(head) if @install + FileUtils.cp_r(head, base) + restore_implementation(base, scratch) + selected.each_with_index do |(file, tests), index| + head_result = run_file(head, file, tests, "head", index) + base_result = run_file(base, file, tests, "base", index) + compare(file, tests, head_result, base_result) + end + end + end + write_report + @report + end + + private + + def git(*args) + output, status = Open3.capture2e("git", "-C", @root, *args) + raise Error, "git #{args.first} failed: #{output}" unless status.success? + + output + end + + def select_tests + selected = {} + changes = git("diff", "--name-status", "-z", "--find-renames", @base, @head).split("\0") + until changes.empty? + status = changes.shift + old_path = changes.shift + path = status.start_with?("R", "C") ? changes.shift : old_path + @report["changed_files"] << {"status" => status, "path" => path, "old_path" => old_path} + next if status == "D" + next unless path.end_with?("_test.rb", "_spec.rb") + + before = (status == "A") ? "" : git("show", "#{@base}:#{old_path}") + after = git("show", "#{@head}:#{path}") + next if before == after + + if !path.start_with?("test/", "test_engine/") || path.end_with?("_spec.rb") + @report["notes"] << "Unsupported test file: #{path}" + next + end + old = TestDefinitions.new(before) + current = TestDefinitions.new(after) + current.unsupported.each { |reason| @report["notes"] << "#{path}: #{reason}" } + tests = current.tests.filter_map do |key, test| + previous = old.tests[key] + next if previous && previous["source"] == test["source"] + + test.except("source").merge( + "definition" => key, "change" => previous ? "modified" : "added" + ) + end + @report["files"] << {"path" => path, "selected" => tests.length} + selected[path] = tests unless tests.empty? + end + selected + end + + def snapshot(revision, destination) + FileUtils.mkdir_p(destination) + archive = File.join(File.dirname(destination), "#{File.basename(destination)}.tar") + git("archive", "--format=tar", "--output=#{archive}", revision) + _, status = Open3.capture2e("tar", "-xf", archive, "-C", destination) + raise Error, "Could not extract #{revision}" unless status.success? + end + + def specification(directory) + @specifications ||= {} + @specifications[directory] ||= begin + output, status = Bundler.with_unbundled_env do + Open3.capture2e( + RbConfig.ruby, "-rjson", "-e", <<~RUBY, chdir: directory + spec = Gem::Specification.load("view_component.gemspec") + abort "Cannot load gemspec" unless spec + bundler = spec.dependencies.find { |dependency| dependency.name == "bundler" } + puts JSON.generate(files: spec.files, bundler: bundler&.requirement&.as_list) + RUBY + ) + end + raise Error, "Cannot load gemspec: #{output}" unless status.success? + + JSON.parse(output) + end + end + + def packaged_files(directory) + specification(directory).fetch("files").select do |path| + raise Error, "Unsafe packaged path: #{path}" if Pathname.new(path).absolute? || path.split("/").include?("..") + + File.file?(File.join(directory, path)) && + path.start_with?("app/", "lib/") && !path.start_with?("app/views/test_mailer/") + end + end + + def restore_implementation(directory, scratch) + original = File.join(scratch, "original") + snapshot(@base, original) + head_files = packaged_files(directory) + base_files = packaged_files(original) + (head_files | base_files).each do |path| + source = File.join(original, path) + destination = File.join(directory, path) + if File.file?(source) + FileUtils.mkdir_p(File.dirname(destination)) + FileUtils.copy_file(source, destination) + elsif File.file?(destination) + FileUtils.rm(destination) + end + end + @report["implementation_files"] = (head_files | base_files).sort + if git("diff", "--name-only", @base, @head, "--", "Gemfile", "view_component.gemspec", "gemfiles").length.positive? + @report["notes"] << "Dependencies changed. Both runs use the PR's dependencies; this does not compare dependency changes." + end + end + + def run_file(directory, file, tests, version, index) + prefix = File.join(@output, "#{index}-#{version}") + input = "#{prefix}-selection.json" + result = "#{prefix}.json" + log = "#{prefix}.log" + File.write(input, JSON.pretty_generate(tests)) + FileUtils.rm_f([result, "#{result}.tmp"]) + env = test_environment(directory) + status, timed_out = run_process( + env, [*bundle_command(directory), "exec", RbConfig.ruby, RUNNER, file, input, result], directory, log, @timeout + ) + data = File.exist?(result) ? JSON.parse(File.read(result)) : {"tests" => []} + data.merge("timeout" => timed_out, "exit_code" => status&.exitstatus, "log" => File.basename(log)) + end + + def test_environment(directory) + gemfile = ENV.fetch("BUNDLE_GEMFILE", "gemfiles/rails_8.1.gemfile") + if Pathname.new(gemfile).absolute? + gemfile = Pathname.new(gemfile).relative_path_from(Pathname.new(@root)).to_s + end + environment = { + "BUNDLE_GEMFILE" => File.expand_path(gemfile, directory), + "RAILS_ENV" => "test", + "MEASURE_COVERAGE" => nil, + "LOADED_GEMS_DIR" => nil + } + environment["BUNDLE_PATH"] = @bundle_path if @bundle_path + environment + end + + def install_dependencies(directory) + log = File.join(@output, "bundle.log") + env = test_environment(directory) + command = bundle_command(directory) + status, _ = run_process(env, [*command, "check"], directory, log, 30) + return if status&.success? + + version = bundler_version(directory) + requirements = version ? [version] : specification(directory)["bundler"] + if requirements && Gem::Specification.find_all_by_name("bundler", Gem::Requirement.new(requirements)).empty? + status, _ = run_process(env, ["gem", "install", "bundler", "--version", requirements.join(", "), "--no-document"], directory, log, 180) + raise Error, "Bundler #{requirements.join(", ")} installation failed; see #{log}" unless status&.success? + end + command = bundle_command(directory) + status, timed_out = run_process(env, [*command, "install", "--quiet"], directory, log, 300) + raise Error, "Dependency installation #{timed_out ? "timed out" : "failed"}; see #{log}" unless status&.success? + end + + def bundler_version(directory) + lockfile = "#{test_environment(directory).fetch("BUNDLE_GEMFILE")}.lock" + locked = File.read(lockfile)[/^BUNDLED WITH\n\s+(\d+\.\d+\.\d+)\s*$/, 1] if File.file?(lockfile) + return locked if locked + + requirement = specification(directory)["bundler"] + if requirement + Gem::Specification.find_all_by_name("bundler", Gem::Requirement.new(requirement)).max_by(&:version)&.version&.to_s + end + end + + def bundle_command(directory) + version = bundler_version(directory) + version ? ["bundle", "_#{version}_"] : ["bundle"] + end + + def run_process(env, command, directory, log, timeout) + status = nil + timed_out = false + File.open(log, "w") do |io| + pid = Bundler.with_unbundled_env do + Process.spawn(env, *command, chdir: directory, out: io, err: io, pgroup: true) + end + deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout + loop do + waited = Process.waitpid2(pid, Process::WNOHANG) + if waited + status = waited.last + break + end + if Process.clock_gettime(Process::CLOCK_MONOTONIC) >= deadline + timed_out = true + Process.kill("KILL", -pid) + Process.waitpid(pid) + break + end + sleep 0.05 + end + end + [status, timed_out] + end + + def compare(file, tests, head, base) + head_tests = head.fetch("tests").to_h { |test| [test.fetch("id"), test] } + base_tests = base.fetch("tests").to_h { |test| [test.fetch("id"), test] } + tests.each do |definition| + matches = head_tests.values.select { |test| test["definition"] == definition["definition"] } + if matches.empty? + @report["tests"] << definition.merge("file" => file, "status" => "inconclusive", + "reason" => head["timeout"] ? "Head timed out" : "Test did not run on head", "head_log" => head["log"]) + end + matches.each do |head_test| + base_test = base_tests[head_test["id"]] + status, reason = classify(head_test, base_test, head, base) + @report["tests"] << definition.merge( + "file" => file, "id" => head_test["id"], "status" => status, "reason" => reason, + "head" => head_test, "base" => base_test, "head_log" => head["log"], "base_log" => base["log"] + ) + end + end + end + + def classify(head_test, base_test, head, base) + return ["inconclusive", "Head timed out"] if head["timeout"] + return ["inconclusive", "Head test run did not complete"] unless head["completed"] + return ["inconclusive", "Head process did not complete normally"] unless [0, 1].include?(head["exit_code"]) + if head["exit_code"] != 0 && head["tests"].none? { |test| ["failed", "error"].include?(test["status"]) } + return ["inconclusive", "Head failed outside the selected tests"] + end + return ["head_failed", "Test does not pass on head"] if ["failed", "error"].include?(head_test["status"]) + return ["inconclusive", "Test skipped or made no assertions on head"] if head_test["status"] != "passed" || head_test["assertions"].zero? + return ["inconclusive", "Base timed out"] if base["timeout"] + return ["inconclusive", "Base test run did not complete"] unless base["completed"] + return ["inconclusive", "Base process did not complete normally"] unless [0, 1].include?(base["exit_code"]) + if base["exit_code"] != 0 && base["tests"].none? { |test| ["failed", "error"].include?(test["status"]) } + return ["inconclusive", "Base failed outside the selected tests"] + end + return ["inconclusive", "Test did not run on base"] unless base_test + + case base_test["status"] + when "failed" then ["demonstrated", "Passes on head; assertion fails without the implementation"] + when "passed" then ["not_demonstrated", "Also passes without the implementation"] + else ["inconclusive", "Base #{base_test["status"]}: inspect the recorded exception"] + end + end + + def write_report + @report["counts"] = @report["tests"].group_by { |test| test["status"] }.transform_values(&:length) + File.write(File.join(@output, "results.json"), JSON.pretty_generate(@report) + "\n") + lines = [ + "## Regression check", "", + "Base: `#{@base}`; head: `#{@head}`.", "", + "| Test | Change | Result | Detail |", + "| --- | --- | --- | --- |" + ] + @report["tests"].each do |test| + values = [test["id"] || "#{test["file"]}:#{test["line"]}", test["change"], test["status"], test["reason"]] + lines << "| #{values.map { |value| RegressionCheck.table_cell(value) }.join(" | ")} |" + end + lines << "" << "No added or modified tests were selected." if @report["tests"].empty? + @report["notes"].each { |note| lines << "" << "- #{note}" } + lines << "" << "Assertion failures demonstrate a difference, not necessarily a bug. Errors, skips, timeouts, and missing results are inconclusive." + summary = lines.join("\n") + "\n" + File.write(File.join(@output, "summary.md"), summary) + puts summary + end + end + + def self.table_cell(value) + value.to_s.gsub(/[\\|]/) { |character| "\\#{character}" }.tr("\n", " ") + end + + def self.main(argv) + options = {head: "HEAD", output: "tmp/regression-check", timeout: 90} + strict = false + OptionParser.new do |parser| + parser.banner = "Usage: ruby script/regression_check.rb --base REV [options]" + parser.on("--base REV") { |value| options[:base] = value } + parser.on("--head REV") { |value| options[:head] = value } + parser.on("--output DIR") { |value| options[:output] = value } + parser.on("--timeout SECONDS", Integer) { |value| options[:timeout] = value } + parser.on("--install") { options[:install] = true } + parser.on("--strict") { strict = true } + end.parse!(argv) + raise Error, "--base is required" unless options[:base] + raise Error, "--timeout must be positive" unless options[:timeout].positive? + + exit_code(Check.new(**options).run, strict: strict) + rescue Error, OptionParser::ParseError => error + warn error.message + 1 + end + + def self.exit_code(report, strict:) + return 0 unless strict + + (report["tests"].any? && report["tests"].all? { |test| test["status"] == "demonstrated" } && report["notes"].empty?) ? 0 : 1 + end +end + +exit RegressionCheck.main(ARGV) if $PROGRAM_NAME == __FILE__ diff --git a/script/regression_check/runner.rb b/script/regression_check/runner.rb new file mode 100644 index 000000000..77a79b850 --- /dev/null +++ b/script/regression_check/runner.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require "json" +require "minitest" + +file, selection_path, result_path = ARGV +ARGV.clear +selection = JSON.parse(File.read(selection_path)) +root = Dir.pwd +$LOAD_PATH.unshift(File.join(root, "lib")) +$LOAD_PATH.unshift(File.join(root, file.start_with?("test_engine/") ? "test_engine" : "test")) +$LOAD_PATH.unshift(File.join(root, "test_engine/lib")) if file.start_with?("test_engine/") +require File.join(root, file) + +definitions = {} +Minitest.seed = 0 +selected_methods = Minitest::Runnable.runnables.to_h do |klass| + selected = klass.runnable_methods.select do |name| + source_file, line = klass.instance_method(name).source_location + next unless source_file && File.expand_path(source_file) == File.join(root, file) + + definition = selection.find { |test| test["name"] == name && (test["line"]..test["end_line"]).cover?(line) } + definitions["#{klass.name}##{name}"] = definition["definition"] if definition + definition + end + [klass, selected] +end +selected_methods.each { |klass, methods| klass.define_singleton_method(:runnable_methods) { methods } } + +reporter_class = Class.new(Minitest::AbstractReporter) do + define_method(:initialize) do + @results = [] + end + + define_method(:record) do |result| + id = "#{result.class_name}##{result.name}" + status = if result.skipped? + "skipped" + elsif result.failures.any? { |failure| failure.is_a?(Minitest::UnexpectedError) } + "error" + elsif result.passed? + "passed" + else + "failed" + end + @results << { + "id" => id, "definition" => definitions.fetch(id), "status" => status, + "assertions" => result.assertions, + "failures" => result.failures.map { |failure| {"class" => failure.class.name, "message" => failure.message} } + } + write_results(false) + end + + define_method(:report) do + write_results(true) + end + + define_method(:write_results) do |completed| + File.write("#{result_path}.tmp", JSON.pretty_generate("tests" => @results, "completed" => completed)) + File.rename("#{result_path}.tmp", result_path) + end +end + +File.write(result_path, JSON.generate("tests" => [])) +Minitest.singleton_class.define_method(:plugin_regression_check_init) do |_options| + reporter << reporter_class.new +end +Minitest.extensions << "regression_check" +ARGV.replace(["--seed", "0"]) +Minitest.autorun diff --git a/script/regression_check_history.rb b/script/regression_check_history.rb new file mode 100644 index 000000000..9cdb0cb89 --- /dev/null +++ b/script/regression_check_history.rb @@ -0,0 +1,116 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require_relative "regression_check" + +options = {limit: 100, output: "tmp/regression-history", timeout: 90} +OptionParser.new do |parser| + parser.banner = "Usage: ruby script/regression_check_history.rb [options]" + parser.on("--limit NUMBER", Integer) { |value| options[:limit] = value } + parser.on("--output DIR") { |value| options[:output] = value } + parser.on("--timeout SECONDS", Integer) { |value| options[:timeout] = value } +end.parse! +abort "--limit must be between 1 and 100" unless (1..100).cover?(options[:limit]) + +output = File.expand_path(options[:output]) +FileUtils.mkdir_p(output) +metadata = File.join(output, "pull_requests.json") +unless File.exist?(metadata) + json, status = Open3.capture2e( + "gh", "api", "--method", "GET", "repos/ViewComponent/view_component/pulls", + "-f", "state=all", "-f", "sort=created", "-f", "direction=desc", "-f", "per_page=#{options[:limit]}" + ) + abort json unless status.success? + pulls = JSON.parse(json).map do |pr| + pr.slice("number", "title", "html_url", "state", "merged_at", "created_at").merge( + "base_sha" => pr.fetch("base").fetch("sha"), "head_sha" => pr.fetch("head").fetch("sha") + ) + end + File.write(metadata, JSON.pretty_generate(pulls) + "\n") +end +pulls = JSON.parse(File.read(metadata)).first(options[:limit]) +pulls.each_slice(10) do |batch| + refs = batch.map { |pr| "+refs/pull/#{pr.fetch("number")}/head:refs/regression-check/#{pr.fetch("number")}" } + log, status = Open3.capture2e("git", "fetch", "--no-tags", "origin", *refs) + abort log unless status.success? +end + +results = pulls.map do |pr| + directory = File.join(output, pr.fetch("number").to_s) + result_path = File.join(directory, "results.json") + report = File.exist?(result_path) ? JSON.parse(File.read(result_path)) : nil + unless report && report["checker_digest"] == RegressionCheck::CHECKER_DIGEST && + report["head"] == pr.fetch("head_sha") + puts "Checking ##{pr.fetch("number")}: #{pr.fetch("title")}" + begin + [pr.fetch("head_sha"), pr.fetch("base_sha")].each do |sha| + _, status = Open3.capture2e("git", "cat-file", "-e", "#{sha}^{commit}") + unless status.success? + log, status = Open3.capture2e("git", "fetch", "--no-tags", "origin", sha) + raise RegressionCheck::Error, log unless status.success? + end + end + report = RegressionCheck::Check.new( + base: pr.fetch("base_sha"), head: pr.fetch("head_sha"), output: directory, + timeout: options[:timeout], install: true + ).run + rescue RegressionCheck::Error => error + report = { + "checker_digest" => RegressionCheck::CHECKER_DIGEST, "head" => pr.fetch("head_sha"), + "tests" => [], "counts" => {}, "notes" => [], "error" => error.message + } + FileUtils.mkdir_p(directory) + File.write(result_path, JSON.pretty_generate(report) + "\n") + warn "##{pr.fetch("number")}: #{error.message}" + end + end + pr.merge("report" => report) +end + +File.write(File.join(output, "results.json"), JSON.pretty_generate(results) + "\n") +counts = results.flat_map { |pr| pr.fetch("report").fetch("tests") }.group_by { |test| test.fetch("status") }.transform_values(&:length) +selected_prs = results.count { |pr| pr.fetch("report").fetch("tests").any? } +errors = results.count { |pr| pr.fetch("report")["error"] } +lines = [ + "# Historical regression check", "", + "Latest #{pulls.length} pull requests by creation time, including open and closed pull requests. PR metadata is frozen in `pull_requests.json`.", "", + "Ruby #{RUBY_VERSION}; Rails 8.1 appraisal from each PR. Tests use each PR's dependencies.", "", + "#{selected_prs} PRs had selected tests; #{errors} PRs could not complete dependency setup.", "", + "Test results: #{counts.map { |status, count| "#{count} #{status}" }.join(", ")}.", "", + "| PR | Demonstrated | Also passes on base | Inconclusive | Head failed | Notes |", + "| --- | --- | --- | --- | --- | --- |" +] +results.each do |pr| + report = pr.fetch("report") + totals = report.fetch("counts") + notes = (report["error"] || report.fetch("notes").join("; ")).gsub(RegressionCheck::ROOT + "/", "") + notes = "No added or modified tests" if report.fetch("tests").empty? && notes.empty? + values = [ + "[##{pr.fetch("number")}](#{pr.fetch("html_url")})", + *%w[demonstrated not_demonstrated inconclusive head_failed].map { |key| totals.fetch(key, 0) }, + notes + ] + lines << "| #{values.map { |value| RegressionCheck.table_cell(value) }.join(" | ")} |" +end +lines << "" << "## Tests that also pass without the implementation" << "" +results.each do |pr| + tests = pr.fetch("report").fetch("tests").select { |test| test.fetch("status") == "not_demonstrated" } + next if tests.empty? + + lines << "- [##{pr.fetch("number")}](#{pr.fetch("html_url")}): #{tests.map { |test| "`#{test.fetch("id")}` (#{test.fetch("change")})" }.join(", ")}" +end +lines << "" << "## Implementation changes without selected tests" << "" +results.each do |pr| + report = pr.fetch("report") + next unless report.fetch("tests").empty? && !report["error"] + + paths = report.fetch("changed_files", []).map { |file| file.fetch("path") }.select do |path| + path.start_with?("app/", "lib/") && !path.start_with?("app/views/test_mailer/") + end + next if paths.empty? + + lines << "- [##{pr.fetch("number")}](#{pr.fetch("html_url")}): #{paths.map { |path| "`#{path}`" }.join(", ")}" +end +lines << "" << "Passing on base is a review signal, not proof of missing coverage. Pre-fix exceptions, unsupported definitions, dependency problems, and historical head failures require manual review." +File.write(File.join(output, "summary.md"), lines.join("\n") + "\n") +puts lines.join("\n") diff --git a/test/script/regression_check_test.rb b/test/script/regression_check_test.rb new file mode 100644 index 000000000..ed4d1ea8d --- /dev/null +++ b/test/script/regression_check_test.rb @@ -0,0 +1,388 @@ +# frozen_string_literal: true + +require "minitest/autorun" +require_relative "../../script/regression_check" + +class RegressionCheckTest < Minitest::Test + def test_escapes_markdown_table_cells + assert_equal "\\\\", RegressionCheck.table_cell("\\") + assert_equal "\\|", RegressionCheck.table_cell("|") + assert_equal ("\\" * 3) + "| next", RegressionCheck.table_cell("\\|\nnext") + end + + def test_parses_methods_and_rails_test_blocks_without_matching_strings + definitions = RegressionCheck::TestDefinitions.new(<<~RUBY) + module Examples + class ExampleTest + def test_method + assert true + end + test "a useful test" do + assert true + end + def self.test_not_an_instance_method; end + def helper; end + text = "def test_not_really_a_test" + def test_hidden_later; end + private :test_hidden_later + private def test_inline_private; end + private + def test_private_helper; end + end + end + RUBY + + assert_equal ["Examples::ExampleTest#test_method", "Examples::ExampleTest#test_a_useful_test"], definitions.tests.keys + assert_empty definitions.unsupported + end + + def test_reports_dynamic_and_rspec_definitions + definitions = RegressionCheck::TestDefinitions.new(<<~RUBY) + class ExampleTest + define_method("test_dynamic") { assert true } + test(name) { assert true } + it("does something") { expect(true).to eq(true) } + end + RUBY + + assert_equal 3, definitions.unsupported.length + end + + def test_rejects_invalid_ruby + assert_raises(RegressionCheck::Error) { RegressionCheck::TestDefinitions.new("class") } + end + + def test_demonstrates_only_the_added_test_and_preserves_mailer_fixtures + with_repository do + write("app/views/test_mailer/example.html.erb", "fixture") + write("lib/behavior.rb", 'VALUE = "new"') + add_test('assert_equal "new", VALUE; assert_equal "fixture", File.read("app/views/test_mailer/example.html.erb")') + report = check + + assert_equal({"demonstrated" => 1}, report["counts"]) + assert_equal "ExampleTest#test_regression", report["tests"].first["id"] + assert_equal "failed", report["tests"].first.dig("base", "status") + assert_equal 'VALUE = "new"', File.read(File.join(@repo, "lib/behavior.rb")) + assert_empty Dir[File.join(@repo, "tmp/regression-check-*")] + end + end + + def test_reports_tests_that_also_pass_on_base + with_repository do + add_test("assert true") + + assert_equal({"not_demonstrated" => 1}, check["counts"]) + end + end + + def test_checks_modified_existing_tests + with_repository do + write("test/example_test.rb", test_source("assert true")) + @base = commit + write("lib/behavior.rb", 'VALUE = "new"') + write("test/example_test.rb", test_source('assert_equal "new", VALUE')) + report = check + + assert_equal({"demonstrated" => 1}, report["counts"]) + assert_equal "modified", report["tests"].first["change"] + end + end + + def test_errors_are_not_treated_as_demonstrated_regressions + with_repository do + write("lib/behavior.rb", 'VALUE = "new"') + add_test('raise "missing feature" if VALUE == "old"; assert true') + report = check + + assert_equal({"inconclusive" => 1}, report["counts"]) + assert_equal "error", report["tests"].first.dig("base", "status") + end + end + + def test_head_assertion_failures_are_not_regressions + with_repository do + add_test('assert_equal "new", VALUE') + + assert_equal({"head_failed" => 1}, check["counts"]) + end + end + + def test_skips_and_zero_assertions_are_inconclusive + with_repository do + add_test('skip "unsupported"') + assert_equal({"inconclusive" => 1}, check["counts"]) + + add_test("nil") + assert_equal({"inconclusive" => 1}, check["counts"]) + end + end + + def test_boot_errors_and_stale_results_cannot_pass + with_repository do + write("lib/behavior.rb", 'VALUE = "new"') + add_test('assert_equal "new", VALUE') + assert_equal({"demonstrated" => 1}, check["counts"]) + write("test/example_test.rb", "raise 'boot failure'\n" + test_source('assert_equal "new", VALUE')) + + assert_equal({"inconclusive" => 1}, check["counts"]) + end + end + + def test_zero_test_runs_are_inconclusive + with_repository do + add_test("assert true", prefix: "if false\n", suffix: "\nend") + + assert_equal({"inconclusive" => 1}, check["counts"]) + end + end + + def test_timeouts_are_inconclusive + with_repository do + add_test("sleep 30; assert true") + report = check(timeout: 0.2) + + assert_equal({"inconclusive" => 1}, report["counts"]) + assert_match(/timed out/, report["tests"].first["reason"]) + end + end + + def test_runs_normal_exit_hooks_without_rerunning_tests + with_repository do + write("test/example_test.rb", test_source(<<~RUBY.chomp)) + at_exit { puts "cleanup ran" } + assert true + end + def test_only_runs_once + refute File.exist?("already_ran.txt") + File.write("already_ran.txt", "ran") + RUBY + report = check + + assert_equal({"not_demonstrated" => 2}, report["counts"]) + assert report["tests"].all? { |test| test["head"]["assertions"] == 1 } + assert_includes File.read(File.join(@repo, "tmp/results/0-head.log")), "cleanup ran" + end + end + + def test_restores_deleted_implementation_and_removes_new_implementation_files + with_repository do + write("lib/deleted.rb", "old") + @base = commit + FileUtils.rm(File.join(@repo, "lib/deleted.rb")) + write("lib/added.rb", "new") + add_test('refute File.exist?("lib/deleted.rb"); assert File.exist?("lib/added.rb")') + + assert_equal({"demonstrated" => 1}, check["counts"]) + end + end + + def test_restores_packaged_app_implementation + with_repository do + write("app/example.rb", "old") + @base = commit + write("app/example.rb", "new") + add_test('assert_equal "new", File.read("app/example.rb")') + + assert_equal({"demonstrated" => 1}, check["counts"]) + end + end + + def test_file_renames_do_not_select_unchanged_tests + with_repository do + FileUtils.mv(File.join(@repo, "test/example_test.rb"), File.join(@repo, "test/renamed_test.rb")) + + assert_empty check["tests"] + end + end + + def test_unsupported_specs_are_explicit + with_repository do + write("spec/example_spec.rb", 'describe("example") { it("works") { expect(true).to eq(true) } }') + report = check + + assert_empty report["tests"] + assert_includes report["notes"], "Unsupported test file: spec/example_spec.rb" + end + end + + def test_uses_merge_base_rather_than_unrelated_base_branch_changes + with_repository do + git("checkout", "-b", "other") + write("lib/behavior.rb", 'VALUE = "unrelated"') + other = commit + git("checkout", "-b", "feature", @base) + write("lib/behavior.rb", 'VALUE = "new"') + add_test('assert_equal "new", VALUE') + report = check(base: other) + + assert_equal @base, report["base"] + assert_equal({"demonstrated" => 1}, report["counts"]) + end + end + + def test_selects_tests_independently_when_they_share_a_file + with_repository do + write("lib/behavior.rb", 'VALUE = "new"') + write("test/example_test.rb", test_source(<<~RUBY.chomp)) + assert_equal "new", VALUE + end + def test_also_passes + assert true + RUBY + report = check + + assert_equal({"demonstrated" => 1, "not_demonstrated" => 1}, report["counts"]) + end + end + + def test_runs_literal_rails_style_test_blocks + with_repository do + write("lib/behavior.rb", 'VALUE = "new"') + write("test/example_test.rb", <<~RUBY) + require "minitest/autorun" + require "behavior" + class ExampleTest < Minitest::Test + def self.test(name, &block) + define_method("test_\#{name.gsub(/\\s+/, "_")}", &block) + end + test "a literal description" do + assert_equal "new", VALUE + end + end + RUBY + + assert_equal({"demonstrated" => 1}, check["counts"]) + end + end + + def test_selects_the_engine_test_bootstrap + with_repository do + write("lib/behavior.rb", 'VALUE = "new"') + write("test_engine/test_helper.rb", "ENGINE_BOOTSTRAPPED = true\n") + write("test_engine/test/example_test.rb", "require 'test_helper'\n" + + test_source('assert ENGINE_BOOTSTRAPPED; assert_equal "new", VALUE')) + + assert_equal({"demonstrated" => 1}, check["counts"]) + end + end + + def test_reuses_the_configured_bundle_outside_the_temporary_checkout + with_repository do + Bundler.settings.temporary(path: "vendor/bundle") do + checker = RegressionCheck::Check.new(root: @repo, base: @base, output: File.join(@repo, "tmp/results")) + environment = checker.send(:test_environment, File.join(@repo, "tmp/head")) + + assert_equal File.join(Bundler.root, "vendor/bundle"), environment["BUNDLE_PATH"] + end + end + end + + def test_explicit_strict_mode_requires_demonstrated_regressions + with_repository do + add_test("assert true") + head = commit + previous = ENV["BUNDLE_GEMFILE"] + ENV["BUNDLE_GEMFILE"] = File.join(@repo, "Gemfile") + checker = RegressionCheck::Check.new( + root: @repo, base: @base, head: head, output: File.join(@repo, "tmp/results") + ) + report = nil + capture_io { report = checker.run } + assert_equal 0, RegressionCheck.exit_code(report, strict: false) + assert_equal 1, RegressionCheck.exit_code(report, strict: true) + assert_equal 1, RegressionCheck.exit_code({"tests" => [], "notes" => []}, strict: true) + report["tests"].first["status"] = "demonstrated" + assert_equal 0, RegressionCheck.exit_code(report, strict: true) + report["notes"] << "Unsupported test definition" + assert_equal 1, RegressionCheck.exit_code(report, strict: true) + ensure + ENV["BUNDLE_GEMFILE"] = previous + end + end + + private + + def with_repository + FileUtils.mkdir_p(File.join(RegressionCheck::ROOT, "tmp")) + Dir.mktmpdir("regression-fixture-", File.join(RegressionCheck::ROOT, "tmp")) do |directory| + @repo = directory + git("init", "--quiet") + git("config", "user.name", "Regression checker") + git("config", "user.email", "regression-check@example.invalid") + write(".gitignore", "/tmp/\n") + write("Gemfile", "source 'https://rubygems.org'\ngem 'minitest', '#{Minitest::VERSION}'\n") + write("view_component.gemspec", <<~RUBY) + Gem::Specification.new do |spec| + spec.name = "view_component" + spec.version = "0.0.0" + spec.summary = "Regression check fixture" + spec.authors = ["Regression checker"] + spec.files = Dir["lib/**/*", "app/**/*"] + end + RUBY + write("lib/behavior.rb", 'VALUE = "old"') + write("test/example_test.rb", test_source('flunk "unrelated existing failure"', name: "test_existing")) + environment = {"BUNDLE_GEMFILE" => File.join(@repo, "Gemfile")} + bundle_path = Bundler.settings[:path] + environment["BUNDLE_PATH"] = File.expand_path(bundle_path, Bundler.root) if bundle_path + output, status = Bundler.with_unbundled_env do + Open3.capture2e(environment, "bundle", "lock", "--local", chdir: @repo) + end + raise output unless status.success? + + @base = commit + yield + end + end + + def write(path, content) + destination = File.join(@repo, path) + FileUtils.mkdir_p(File.dirname(destination)) + File.write(destination, content) + end + + def test_source(body, name: "test_regression") + <<~RUBY + require "minitest/autorun" + require "behavior" + class ExampleTest < Minitest::Test + def #{name} + #{body} + end + end + RUBY + end + + def add_test(body, prefix: "", suffix: "") + write("test/example_test.rb", prefix + test_source('flunk "unrelated existing failure"', name: "test_existing") + + test_source(body) + suffix) + end + + def git(*args) + output, status = Open3.capture2e("git", "-C", @repo, *args) + raise output unless status.success? + + output.strip + end + + def commit + git("add", ".") + git("-c", "commit.gpgsign=false", "commit", "--quiet", "-m", "Fixture") + git("rev-parse", "HEAD") + end + + def check(base: @base, **options) + head = commit + previous = ENV["BUNDLE_GEMFILE"] + ENV["BUNDLE_GEMFILE"] = "Gemfile" + report = nil + capture_io do + report = RegressionCheck::Check.new( + root: @repo, base: base, head: head, output: File.join(@repo, "tmp/results"), **options + ).run + end + report + ensure + ENV["BUNDLE_GEMFILE"] = previous + end +end From 3ce9a88540429fbb4028bd8fe0037071bfde550e Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 27 Oct 2026 08:26:36 -0600 Subject: [PATCH 2/2] Pin setup-ruby to an exact version in the regression check --- .github/workflows/regression-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/regression-check.yml b/.github/workflows/regression-check.yml index 698990cf9..b40afc0ac 100644 --- a/.github/workflows/regression-check.yml +++ b/.github/workflows/regression-check.yml @@ -20,7 +20,7 @@ jobs: fetch-depth: 0 persist-credentials: false - name: Setup Ruby - uses: ruby/setup-ruby@26e5b7d197ebf60545ee9667bcbace0be11ccaec # v1 + uses: ruby/setup-ruby@7f797815d8adceb1c412e7abefdd1ab4abc47a5f # v1.335.0 with: ruby-version: 4.0 bundler-cache: true