diff --git a/.github/workflows/regression-check.yml b/.github/workflows/regression-check.yml new file mode 100644 index 000000000..b40afc0ac --- /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@7f797815d8adceb1c412e7abefdd1ab4abc47a5f # v1.335.0 + 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 9e9eb782e..2adbb655e 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* + * Speed up rendering with direct compiled-template calls and avoid intermediate method allocations when invoking lambda-backed slots. *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