Merge pull request #382 from Shopify/rwstauner/tests

Fix up tests and run them on CI

.github/workflows/test.yml

@@ -0,0 +1,33 @@
+name: Test
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        working-directory: yjit-metrics
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          path: yjit-metrics
+
+      - name: Checkout yjit-bench
+        uses: actions/checkout@v4
+        with:
+          repository: Shopify/yjit-bench
+          path: yjit-bench
+
+      - uses: ruby/setup-ruby@v1
+        with:
+          bundler-cache: true
+          working-directory: yjit-metrics
+
+      - name: Run tests
+        run: bundle exec rake

Gemfile

@@ -9,5 +9,5 @@ gem "kramdown" # used in reporting
 gem "sass-embedded"
 gem "slack-ruby-client"
 
-gem "minitest", "~>5.11.3"
+gem "minitest"
 gem "webrick"

Gemfile.lock

@@ -24,7 +24,7 @@ GEM
       rb-fsevent (~> 0.10, >= 0.10.3)
       rb-inotify (~> 0.9, >= 0.9.10)
     logger (1.6.5)
-    minitest (5.11.3)
+    minitest (5.25.4)
     multipart-post (2.4.1)
     net-http (0.4.1)
       uri
@@ -54,7 +54,7 @@ DEPENDENCIES
   kramdown
   listen
   logger
-  minitest (~> 5.11.3)
+  minitest
   rake
   sass-embedded
   slack-ruby-client

lib/yjit_metrics.rb

@@ -411,7 +411,7 @@ def run_single_benchmark(benchmark_info, harness_settings:, shell_settings:,
       result = ErrorData.new(
         exit_status: script_details[:exit_status],
         error: "Failure in benchmark test harness, exit status: #{script_details[:exit_status].inspect}",
-        summary: script_details[:stderr]&.lines&.detect { |l| l.match?(/\S/) }&.sub("#{Dir.pwd}", ".")&.strip,
+        summary: summarize_failure_output(script_details[:stderr])
       )
 
       STDERR.puts "-----"
@@ -535,4 +535,14 @@ def merge_benchmark_data(all_run_data)
 
     return bench_data
   end
+
+  # Try to find the first relevant error line from a stderr string.
+  def summarize_failure_output(stderr)
+    return unless stderr
+
+    stderr.lines
+      .reject { |l| l.match?(%r{^/.+?\.rb:\d+: warning: }) }
+      .detect { |l| l.match?(/\S/) }&.sub("#{Dir.pwd}", ".")
+      &.strip
+  end
 end

lib/yjit_metrics/report.rb

@@ -61,7 +61,7 @@ def filter_benchmark_names(names)
     # Take column headings, formats for the percent operator and data, and arrange it
     # into a simple ASCII table returned as a string.
     def format_as_table(headings, col_formats, data, separator_character: "-", column_spacer: "  ")
-      out = ""
+      out = +""
 
       unless data && data[0] && col_formats && col_formats[0] && headings && headings[0]
         $stderr.puts "Error in format_as_table..."

lib/yjit_metrics/reports/per_bench_ruby_comparison_report.rb

@@ -10,7 +10,7 @@
 # The first configuration given is assumed to be the baseline against
 # which the other configs are measured.
 module YJITMetrics
-  class PerBenchRubyComparisonRepor < Report
+  class PerBenchRubyComparisonReport < Report
     def self.report_name
       "per_bench_compare"
     end

test/benchmark_mocked_test.rb

@@ -166,7 +166,7 @@ def test_single_benchmark_failure
             run_script: fake_runner)
 
         refute_predicate result, :success?
-        assert_equal -1, result.exit_status
+        assert_equal(-1, result.exit_status)
     end
 
 end

test/reporting_test.rb

@@ -17,7 +17,7 @@ def test_creating_per_bench_report
         results.add_for_config "with_yjit", JSON.load(File.read "test/data/2021-09-13-100043_basic_benchmark_prod_ruby_with_yjit.json")
         results.add_for_config "yjit_stats", JSON.load(File.read "test/data/2021-09-13-100043_basic_benchmark_yjit_stats.json")
 
-        report = YJITMetrics::PerBenchRubyComparison.new [ "no_jit", "with_yjit", "yjit_stats" ], results
+        report = YJITMetrics::PerBenchRubyComparisonReport.new [ "no_jit", "with_yjit", "yjit_stats" ], results
         report.to_s
     end