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

Fix up tests and run them on CI #382

Merged
merged 6 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -54,7 +54,7 @@ DEPENDENCIES
kramdown
listen
logger
minitest (~> 5.11.3)
minitest
rake
sass-embedded
slack-ruby-client
Expand Down
12 changes: 11 additions & 1 deletion lib/yjit_metrics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 "-----"
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/yjit_metrics/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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..."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/benchmark_mocked_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/reporting_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading