From f8ae3eace478ec48d9bcff1d43d9ab62021ef021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 18 Nov 2025 04:42:40 +0000 Subject: [PATCH 1/8] Move logic to build the output path to BenchmarkRunner --- lib/benchmark_runner.rb | 58 ++++++++------ run_benchmarks.rb | 9 +-- test/benchmark_runner_test.rb | 102 ++++++++++++++---------- test/run_benchmarks_integration_test.rb | 7 +- 4 files changed, 102 insertions(+), 74 deletions(-) diff --git a/lib/benchmark_runner.rb b/lib/benchmark_runner.rb index 754afd8e..dcee1224 100644 --- a/lib/benchmark_runner.rb +++ b/lib/benchmark_runner.rb @@ -6,37 +6,49 @@ # Extracted helper methods from run_benchmarks.rb for testing module BenchmarkRunner - module_function + class << self + # Determine output path - either use the override or find a free file number + def output_path(out_path_dir, out_override: nil) + if out_override + out_override + else + # If no out path is specified, find a free file index for the output files + file_no = free_file_no(out_path_dir) + File.join(out_path_dir, "output_%03d" % file_no) + end + end - # Find the first available file number for output files - def free_file_no(directory) - (1..).each do |file_no| - out_path = File.join(directory, "output_%03d.csv" % file_no) - return file_no unless File.exist?(out_path) + # Render a graph from JSON benchmark data + def render_graph(json_path) + png_path = json_path.sub(/\.json$/, '.png') + require_relative 'graph_renderer' + GraphRenderer.render(json_path, png_path) end - end - # Render a graph from JSON benchmark data - def render_graph(json_path) - png_path = json_path.sub(/\.json$/, '.png') - require_relative 'graph_renderer' - GraphRenderer.render(json_path, png_path) - end + # Checked system - error or return info if the command fails + def check_call(command, env: {}, raise_error: true, quiet: false) + puts("+ #{command}") unless quiet - # Checked system - error or return info if the command fails - def check_call(command, env: {}, raise_error: true, quiet: false) - puts("+ #{command}") unless quiet + result = {} - result = {} + result[:success] = system(env, command) + result[:status] = $? - result[:success] = system(env, command) - result[:status] = $? + unless result[:success] + puts "Command #{command.inspect} failed with exit code #{result[:status].exitstatus} in directory #{Dir.pwd}" + raise RuntimeError.new if raise_error + end - unless result[:success] - puts "Command #{command.inspect} failed with exit code #{result[:status].exitstatus} in directory #{Dir.pwd}" - raise RuntimeError.new if raise_error + result end - result + private + + def free_file_no(directory) + (1..).each do |file_no| + out_path = File.join(directory, "output_%03d.csv" % file_no) + return file_no unless File.exist?(out_path) + end + end end end diff --git a/run_benchmarks.rb b/run_benchmarks.rb index 8309cdf7..f548e683 100755 --- a/run_benchmarks.rb +++ b/run_benchmarks.rb @@ -66,14 +66,7 @@ ) table, format = builder.build -output_path = nil -if args.out_override - output_path = args.out_override -else - # If no out path is specified, find a free file index for the output files - file_no = BenchmarkRunner.free_file_no(args.out_path) - output_path = File.join(args.out_path, "output_%03d" % file_no) -end +output_path = BenchmarkRunner.output_path(args.out_path, out_override: args.out_override) # Save the raw data as JSON out_json_path = output_path + ".json" diff --git a/test/benchmark_runner_test.rb b/test/benchmark_runner_test.rb index c4203f81..7547d3dd 100644 --- a/test/benchmark_runner_test.rb +++ b/test/benchmark_runner_test.rb @@ -9,46 +9,6 @@ require 'yaml' describe BenchmarkRunner do - describe '.free_file_no' do - it 'returns 1 when no files exist' do - Dir.mktmpdir do |dir| - file_no = BenchmarkRunner.free_file_no(dir) - assert_equal 1, file_no - end - end - - it 'returns next available number when files exist' do - Dir.mktmpdir do |dir| - FileUtils.touch(File.join(dir, 'output_001.csv')) - FileUtils.touch(File.join(dir, 'output_002.csv')) - - file_no = BenchmarkRunner.free_file_no(dir) - assert_equal 3, file_no - end - end - - it 'finds first gap in numbering' do - Dir.mktmpdir do |dir| - FileUtils.touch(File.join(dir, 'output_001.csv')) - FileUtils.touch(File.join(dir, 'output_003.csv')) - - file_no = BenchmarkRunner.free_file_no(dir) - assert_equal 2, file_no - end - end - - it 'handles triple digit numbers' do - Dir.mktmpdir do |dir| - (1..100).each do |i| - FileUtils.touch(File.join(dir, 'output_%03d.csv' % i)) - end - - file_no = BenchmarkRunner.free_file_no(dir) - assert_equal 101, file_no - end - end - end - describe '.check_call' do it 'runs a successful command and returns success status' do result = nil @@ -158,6 +118,68 @@ end end + describe '.output_path' do + it 'returns the override path when provided' do + Dir.mktmpdir do |dir| + override = '/custom/path/output' + result = BenchmarkRunner.output_path(dir, out_override: override) + assert_equal override, result + end + end + + it 'generates path with first available file number when no override' do + Dir.mktmpdir do |dir| + result = BenchmarkRunner.output_path(dir) + expected = File.join(dir, 'output_001') + assert_equal expected, result + end + end + + it 'uses next available file number when files exist' do + Dir.mktmpdir do |dir| + FileUtils.touch(File.join(dir, 'output_001.csv')) + FileUtils.touch(File.join(dir, 'output_002.csv')) + + result = BenchmarkRunner.output_path(dir) + expected = File.join(dir, 'output_003') + assert_equal expected, result + end + end + + it 'finds first gap in numbering when files are non-sequential' do + Dir.mktmpdir do |dir| + FileUtils.touch(File.join(dir, 'output_001.csv')) + FileUtils.touch(File.join(dir, 'output_003.csv')) + + result = BenchmarkRunner.output_path(dir) + expected = File.join(dir, 'output_002') + assert_equal expected, result + end + end + + it 'prefers override even when files exist' do + Dir.mktmpdir do |dir| + FileUtils.touch(File.join(dir, 'output_001.csv')) + + override = '/override/path' + result = BenchmarkRunner.output_path(dir, out_override: override) + assert_equal override, result + end + end + + it 'handles triple digit file numbers' do + Dir.mktmpdir do |dir| + (1..100).each do |i| + FileUtils.touch(File.join(dir, 'output_%03d.csv' % i)) + end + + result = BenchmarkRunner.output_path(dir) + expected = File.join(dir, 'output_101') + assert_equal expected, result + end + end + end + describe '.render_graph' do it 'delegates to GraphRenderer and returns calculated png_path' do Dir.mktmpdir do |dir| diff --git a/test/run_benchmarks_integration_test.rb b/test/run_benchmarks_integration_test.rb index 2c21cb0a..f6bbaa7b 100644 --- a/test/run_benchmarks_integration_test.rb +++ b/test/run_benchmarks_integration_test.rb @@ -34,10 +34,11 @@ File.write(File.join(tmpdir, 'output_001.csv'), 'test') File.write(File.join(tmpdir, 'output_002.csv'), 'test') - # The free_file_no function should find the next number + # The output_path function should find the next number require_relative '../lib/benchmark_runner' - file_no = BenchmarkRunner.free_file_no(tmpdir) - assert_equal 3, file_no + output_path = BenchmarkRunner.output_path(tmpdir) + expected_path = File.join(tmpdir, 'output_003') + assert_equal expected_path, output_path end end From 6d4478b9d570d24fce495625562c3a293dd7d22d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 18 Nov 2025 04:47:16 +0000 Subject: [PATCH 2/8] Extract method to generate the json result --- lib/benchmark_runner.rb | 14 ++++++ run_benchmarks.rb | 10 +--- test/benchmark_runner_test.rb | 87 +++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 9 deletions(-) diff --git a/lib/benchmark_runner.rb b/lib/benchmark_runner.rb index dcee1224..eebf6a2b 100644 --- a/lib/benchmark_runner.rb +++ b/lib/benchmark_runner.rb @@ -18,6 +18,20 @@ def output_path(out_path_dir, out_override: nil) end end + # Write benchmark data to JSON file + def write_json(output_path, ruby_descriptions, bench_data) + out_json_path = output_path + ".json" + File.open(out_json_path, "w") do |file| + out_data = { + metadata: ruby_descriptions, + raw_data: bench_data, + } + json_str = JSON.generate(out_data) + file.write json_str + end + out_json_path + end + # Render a graph from JSON benchmark data def render_graph(json_path) png_path = json_path.sub(/\.json$/, '.png') diff --git a/run_benchmarks.rb b/run_benchmarks.rb index f548e683..22413a84 100755 --- a/run_benchmarks.rb +++ b/run_benchmarks.rb @@ -69,15 +69,7 @@ output_path = BenchmarkRunner.output_path(args.out_path, out_override: args.out_override) # Save the raw data as JSON -out_json_path = output_path + ".json" -File.open(out_json_path, "w") do |file| - out_data = { - metadata: ruby_descriptions, - raw_data: bench_data, - } - json_str = JSON.generate(out_data) - file.write json_str -end +out_json_path = BenchmarkRunner.write_json(output_path, ruby_descriptions, bench_data) # Save data as CSV so we can produce tables/graphs in a spreasheet program # NOTE: we don't do any number formatting for the output file because diff --git a/test/benchmark_runner_test.rb b/test/benchmark_runner_test.rb index 7547d3dd..eba2b74a 100644 --- a/test/benchmark_runner_test.rb +++ b/test/benchmark_runner_test.rb @@ -180,6 +180,93 @@ end end + describe '.write_json' do + it 'writes JSON file with metadata and raw data' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_001') + ruby_descriptions = { + 'ruby-base' => 'ruby 3.3.0', + 'ruby-yjit' => 'ruby 3.3.0 +YJIT' + } + bench_data = { + 'ruby-base' => { 'fib' => { 'time' => 1.5 } }, + 'ruby-yjit' => { 'fib' => { 'time' => 1.0 } } + } + + result_path = BenchmarkRunner.write_json(output_path, ruby_descriptions, bench_data) + + expected_path = File.join(dir, 'output_001.json') + assert_equal expected_path, result_path + assert File.exist?(expected_path) + + json_content = JSON.parse(File.read(expected_path)) + assert_equal ruby_descriptions, json_content['metadata'] + assert_equal bench_data, json_content['raw_data'] + end + end + + it 'returns the JSON file path' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_test') + result_path = BenchmarkRunner.write_json(output_path, {}, {}) + + assert_equal File.join(dir, 'output_test.json'), result_path + end + end + + it 'handles empty metadata and bench data' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_empty') + + result_path = BenchmarkRunner.write_json(output_path, {}, {}) + + assert File.exist?(result_path) + json_content = JSON.parse(File.read(result_path)) + assert_equal({}, json_content['metadata']) + assert_equal({}, json_content['raw_data']) + end + end + + it 'handles nested benchmark data structures' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_nested') + ruby_descriptions = { 'ruby' => 'ruby 3.3.0' } + bench_data = { + 'ruby' => { + 'benchmark1' => { + 'time' => 1.5, + 'rss' => 12345, + 'iterations' => [1.4, 1.5, 1.6] + } + } + } + + result_path = BenchmarkRunner.write_json(output_path, ruby_descriptions, bench_data) + + json_content = JSON.parse(File.read(result_path)) + assert_equal bench_data, json_content['raw_data'] + end + end + + it 'overwrites existing JSON file' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_overwrite') + + # Write first version + BenchmarkRunner.write_json(output_path, { 'v' => '1' }, { 'd' => '1' }) + + # Write second version + new_metadata = { 'v' => '2' } + new_data = { 'd' => '2' } + result_path = BenchmarkRunner.write_json(output_path, new_metadata, new_data) + + json_content = JSON.parse(File.read(result_path)) + assert_equal new_metadata, json_content['metadata'] + assert_equal new_data, json_content['raw_data'] + end + end + end + describe '.render_graph' do it 'delegates to GraphRenderer and returns calculated png_path' do Dir.mktmpdir do |dir| From 2f2d40a8d0f66820e8c94da2d8c27cad8b265e93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 18 Nov 2025 04:49:32 +0000 Subject: [PATCH 3/8] Simplify the write_json method --- lib/benchmark_runner.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/benchmark_runner.rb b/lib/benchmark_runner.rb index eebf6a2b..0bfdedf5 100644 --- a/lib/benchmark_runner.rb +++ b/lib/benchmark_runner.rb @@ -20,15 +20,12 @@ def output_path(out_path_dir, out_override: nil) # Write benchmark data to JSON file def write_json(output_path, ruby_descriptions, bench_data) - out_json_path = output_path + ".json" - File.open(out_json_path, "w") do |file| - out_data = { - metadata: ruby_descriptions, - raw_data: bench_data, - } - json_str = JSON.generate(out_data) - file.write json_str - end + out_json_path = "#{output_path}.json" + out_data = { + metadata: ruby_descriptions, + raw_data: bench_data, + } + File.write(out_json_path, JSON.generate(out_data)) out_json_path end From b5db6f0a793ddaa0ac6cee2fdd22de446312ab3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 18 Nov 2025 04:52:37 +0000 Subject: [PATCH 4/8] Extract method to generate the csv fike --- lib/benchmark_runner.rb | 18 +++++++ run_benchmarks.rb | 13 +---- test/benchmark_runner_test.rb | 98 +++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 12 deletions(-) diff --git a/lib/benchmark_runner.rb b/lib/benchmark_runner.rb index 0bfdedf5..2b6c260c 100644 --- a/lib/benchmark_runner.rb +++ b/lib/benchmark_runner.rb @@ -29,6 +29,24 @@ def write_json(output_path, ruby_descriptions, bench_data) out_json_path end + # Write benchmark results to CSV file + def write_csv(output_path, ruby_descriptions, table) + out_csv_path = "#{output_path}.csv" + output_rows = [] + ruby_descriptions.each do |key, value| + output_rows.append([key, value]) + end + output_rows.append([]) + output_rows.concat(table) + + CSV.open(out_csv_path, "wb") do |csv| + output_rows.each do |row| + csv << row + end + end + out_csv_path + end + # Render a graph from JSON benchmark data def render_graph(json_path) png_path = json_path.sub(/\.json$/, '.png') diff --git a/run_benchmarks.rb b/run_benchmarks.rb index 22413a84..ea0c04d6 100755 --- a/run_benchmarks.rb +++ b/run_benchmarks.rb @@ -74,18 +74,7 @@ # Save data as CSV so we can produce tables/graphs in a spreasheet program # NOTE: we don't do any number formatting for the output file because # we don't want to lose any precision -output_rows = [] -ruby_descriptions.each do |key, value| - output_rows.append([key, value]) -end -output_rows.append([]) -output_rows.concat(table) -out_tbl_path = output_path + ".csv" -CSV.open(out_tbl_path, "wb") do |csv| - output_rows.each do |row| - csv << row - end -end +BenchmarkRunner.write_csv(output_path, ruby_descriptions, table) # Save the output in a text file that we can easily refer to output_str = "" diff --git a/test/benchmark_runner_test.rb b/test/benchmark_runner_test.rb index eba2b74a..d0a1bdee 100644 --- a/test/benchmark_runner_test.rb +++ b/test/benchmark_runner_test.rb @@ -180,6 +180,104 @@ end end + describe '.write_csv' do + it 'writes CSV file with metadata and table data' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_001') + ruby_descriptions = { + 'ruby-base' => 'ruby 3.3.0', + 'ruby-yjit' => 'ruby 3.3.0 +YJIT' + } + table = [ + ['Benchmark', 'ruby-base', 'ruby-yjit'], + ['fib', '1.5', '1.0'], + ['matmul', '2.0', '1.8'] + ] + + result_path = BenchmarkRunner.write_csv(output_path, ruby_descriptions, table) + + expected_path = File.join(dir, 'output_001.csv') + assert_equal expected_path, result_path + assert File.exist?(expected_path) + + csv_rows = CSV.read(expected_path) + assert_equal 'ruby-base', csv_rows[0][0] + assert_equal 'ruby 3.3.0', csv_rows[0][1] + assert_equal 'ruby-yjit', csv_rows[1][0] + assert_equal 'ruby 3.3.0 +YJIT', csv_rows[1][1] + assert_equal [], csv_rows[2] + assert_equal table, csv_rows[3..5] + end + end + + it 'returns the CSV file path' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_test') + result_path = BenchmarkRunner.write_csv(output_path, {}, []) + + assert_equal File.join(dir, 'output_test.csv'), result_path + end + end + + it 'handles empty metadata and table' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_empty') + + result_path = BenchmarkRunner.write_csv(output_path, {}, []) + + assert File.exist?(result_path) + csv_rows = CSV.read(result_path) + assert_equal [[]], csv_rows + end + end + + it 'handles single metadata entry' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_single') + ruby_descriptions = { 'ruby' => 'ruby 3.3.0' } + table = [['Benchmark', 'Time'], ['test', '1.0']] + + result_path = BenchmarkRunner.write_csv(output_path, ruby_descriptions, table) + + csv_rows = CSV.read(result_path) + assert_equal 'ruby', csv_rows[0][0] + assert_equal 'ruby 3.3.0', csv_rows[0][1] + assert_equal [], csv_rows[1] + assert_equal table, csv_rows[2..3] + end + end + + it 'preserves precision in numeric strings' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_precision') + table = [['Benchmark', 'Time'], ['test', '1.234567890123']] + + result_path = BenchmarkRunner.write_csv(output_path, {}, table) + + csv_rows = CSV.read(result_path) + assert_equal '1.234567890123', csv_rows[2][1] + end + end + + it 'overwrites existing CSV file' do + Dir.mktmpdir do |dir| + output_path = File.join(dir, 'output_overwrite') + + # Write first version + BenchmarkRunner.write_csv(output_path, { 'v1' => 'first' }, [['old']]) + + # Write second version + new_descriptions = { 'v2' => 'second' } + new_table = [['new']] + result_path = BenchmarkRunner.write_csv(output_path, new_descriptions, new_table) + + csv_rows = CSV.read(result_path) + assert_equal 'v2', csv_rows[0][0] + assert_equal 'second', csv_rows[0][1] + end + end + end + describe '.write_json' do it 'writes JSON file with metadata and raw data' do Dir.mktmpdir do |dir| From c2c214f6c8238a8ab9fea9af2e715e120cf1ed65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 18 Nov 2025 04:59:22 +0000 Subject: [PATCH 5/8] Improve write_csv method to write directly to CSV without intermediate array --- lib/benchmark_runner.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/benchmark_runner.rb b/lib/benchmark_runner.rb index 2b6c260c..186eea1e 100644 --- a/lib/benchmark_runner.rb +++ b/lib/benchmark_runner.rb @@ -32,18 +32,17 @@ def write_json(output_path, ruby_descriptions, bench_data) # Write benchmark results to CSV file def write_csv(output_path, ruby_descriptions, table) out_csv_path = "#{output_path}.csv" - output_rows = [] - ruby_descriptions.each do |key, value| - output_rows.append([key, value]) - end - output_rows.append([]) - output_rows.concat(table) CSV.open(out_csv_path, "wb") do |csv| - output_rows.each do |row| + ruby_descriptions.each do |key, value| + csv << [key, value] + end + csv << [] + table.each do |row| csv << row end end + out_csv_path end From 5aeda1346a42a3bcddc392e34ffc698501bbb9ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 18 Nov 2025 16:23:07 +0000 Subject: [PATCH 6/8] Extract build_output_text method to BenchmarkRunner module --- lib/benchmark_runner.rb | 23 ++++++ run_benchmarks.rb | 15 +--- test/benchmark_runner_test.rb | 144 ++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 14 deletions(-) diff --git a/lib/benchmark_runner.rb b/lib/benchmark_runner.rb index 186eea1e..d8df7401 100644 --- a/lib/benchmark_runner.rb +++ b/lib/benchmark_runner.rb @@ -3,6 +3,7 @@ require 'csv' require 'json' require 'rbconfig' +require_relative 'table_formatter' # Extracted helper methods from run_benchmarks.rb for testing module BenchmarkRunner @@ -46,6 +47,28 @@ def write_csv(output_path, ruby_descriptions, table) out_csv_path end + # Build output text string with metadata, table, and legend + def build_output_text(ruby_descriptions, table, format, bench_failures, base_name, other_names) + output_str = +"" + + ruby_descriptions.each do |key, value| + output_str << "#{key}: #{value}\n" + end + + output_str << "\n" + output_str << TableFormatter.new(table, format, bench_failures).to_s + "\n" + + unless other_names.empty? + output_str << "Legend:\n" + other_names.each do |name| + output_str << "- #{name} 1st itr: ratio of #{base_name}/#{name} time for the first benchmarking iteration.\n" + output_str << "- #{base_name}/#{name}: ratio of #{base_name}/#{name} time. Higher is better for #{name}. Above 1 represents a speedup.\n" + end + end + + output_str + end + # Render a graph from JSON benchmark data def render_graph(json_path) png_path = json_path.sub(/\.json$/, '.png') diff --git a/run_benchmarks.rb b/run_benchmarks.rb index ea0c04d6..91b10ac3 100755 --- a/run_benchmarks.rb +++ b/run_benchmarks.rb @@ -11,7 +11,6 @@ require_relative 'lib/cpu_config' require_relative 'lib/benchmark_runner' require_relative 'lib/benchmark_suite' -require_relative 'lib/table_formatter' require_relative 'lib/argument_parser' require_relative 'lib/results_table_builder' @@ -77,19 +76,7 @@ BenchmarkRunner.write_csv(output_path, ruby_descriptions, table) # Save the output in a text file that we can easily refer to -output_str = "" -ruby_descriptions.each do |key, value| - output_str << "#{key}: #{value}\n" -end -output_str += "\n" -output_str += TableFormatter.new(table, format, bench_failures).to_s + "\n" -unless other_names.empty? - output_str << "Legend:\n" - other_names.each do |name| - output_str << "- #{name} 1st itr: ratio of #{base_name}/#{name} time for the first benchmarking iteration.\n" - output_str << "- #{base_name}/#{name}: ratio of #{base_name}/#{name} time. Higher is better for #{name}. Above 1 represents a speedup.\n" - end -end +output_str = BenchmarkRunner.build_output_text(ruby_descriptions, table, format, bench_failures, base_name, other_names) out_txt_path = output_path + ".txt" File.open(out_txt_path, "w") { |f| f.write output_str } diff --git a/test/benchmark_runner_test.rb b/test/benchmark_runner_test.rb index d0a1bdee..8c47509c 100644 --- a/test/benchmark_runner_test.rb +++ b/test/benchmark_runner_test.rb @@ -365,6 +365,150 @@ end end + describe '.build_output_text' do + it 'builds output text with metadata, table, and legend' do + ruby_descriptions = { + 'ruby-base' => 'ruby 3.3.0', + 'ruby-yjit' => 'ruby 3.3.0 +YJIT' + } + table = [ + ['bench', 'ruby-base (ms)', 'stddev (%)', 'ruby-yjit (ms)', 'stddev (%)'], + ['fib', '100.0', '5.0', '50.0', '3.0'] + ] + format = ['%s', '%.1f', '%.1f', '%.1f', '%.1f'] + bench_failures = {} + base_name = 'ruby-base' + other_names = ['ruby-yjit'] + + result = BenchmarkRunner.build_output_text( + ruby_descriptions, table, format, bench_failures, base_name, other_names + ) + + assert_includes result, 'ruby-base: ruby 3.3.0' + assert_includes result, 'ruby-yjit: ruby 3.3.0 +YJIT' + assert_includes result, 'Legend:' + assert_includes result, '- ruby-yjit 1st itr: ratio of ruby-base/ruby-yjit time for the first benchmarking iteration.' + assert_includes result, '- ruby-base/ruby-yjit: ratio of ruby-base/ruby-yjit time. Higher is better for ruby-yjit. Above 1 represents a speedup.' + end + + it 'includes formatted table in output' do + ruby_descriptions = { 'ruby' => 'ruby 3.3.0' } + table = [ + ['bench', 'ruby (ms)', 'stddev (%)'], + ['fib', '100.0', '5.0'] + ] + format = ['%s', '%.1f', '%.1f'] + bench_failures = {} + base_name = 'ruby' + other_names = [] + + result = BenchmarkRunner.build_output_text( + ruby_descriptions, table, format, bench_failures, base_name, other_names + ) + + # Should contain table headers + assert_includes result, 'bench' + assert_includes result, 'ruby (ms)' + # Should contain table data + assert_includes result, 'fib' + assert_includes result, '100.0' + end + + it 'omits legend when no other executables' do + ruby_descriptions = { 'ruby' => 'ruby 3.3.0' } + table = [['bench', 'ruby (ms)'], ['fib', '100.0']] + format = ['%s', '%.1f'] + bench_failures = {} + base_name = 'ruby' + other_names = [] + + result = BenchmarkRunner.build_output_text( + ruby_descriptions, table, format, bench_failures, base_name, other_names + ) + + refute_includes result, 'Legend:' + end + + it 'handles multiple other executables in legend' do + ruby_descriptions = { + 'ruby' => 'ruby 3.3.0', + 'ruby-yjit' => 'ruby 3.3.0 +YJIT', + 'ruby-rjit' => 'ruby 3.3.0 +RJIT' + } + table = [['bench', 'ruby (ms)', 'ruby-yjit (ms)', 'ruby-rjit (ms)']] + format = ['%s', '%.1f', '%.1f', '%.1f'] + bench_failures = {} + base_name = 'ruby' + other_names = ['ruby-yjit', 'ruby-rjit'] + + result = BenchmarkRunner.build_output_text( + ruby_descriptions, table, format, bench_failures, base_name, other_names + ) + + assert_includes result, 'ruby-yjit 1st itr' + assert_includes result, 'ruby-rjit 1st itr' + assert_includes result, 'ruby/ruby-yjit' + assert_includes result, 'ruby/ruby-rjit' + end + + it 'includes benchmark failures in formatted output' do + ruby_descriptions = { 'ruby' => 'ruby 3.3.0' } + table = [['bench', 'ruby (ms)'], ['fib', '100.0']] + format = ['%s', '%.1f'] + bench_failures = { + 'ruby' => { 'failed_bench' => 'error message' } + } + base_name = 'ruby' + other_names = [] + + result = BenchmarkRunner.build_output_text( + ruby_descriptions, table, format, bench_failures, base_name, other_names + ) + + # TableFormatter handles displaying failures, just verify it's called + assert_kind_of String, result + assert result.length > 0 + end + + it 'handles empty ruby_descriptions' do + ruby_descriptions = {} + table = [['bench']] + format = ['%s'] + bench_failures = {} + base_name = 'ruby' + other_names = [] + + result = BenchmarkRunner.build_output_text( + ruby_descriptions, table, format, bench_failures, base_name, other_names + ) + + assert_kind_of String, result + assert result.start_with?("\n") # Should start with newline after empty descriptions + end + + it 'preserves order of ruby_descriptions' do + ruby_descriptions = { + 'ruby-a' => 'version A', + 'ruby-b' => 'version B', + 'ruby-c' => 'version C' + } + table = [['bench']] + format = ['%s'] + bench_failures = {} + base_name = 'ruby-a' + other_names = [] + + result = BenchmarkRunner.build_output_text( + ruby_descriptions, table, format, bench_failures, base_name, other_names + ) + + lines = result.lines + assert_includes lines[0], 'ruby-a: version A' + assert_includes lines[1], 'ruby-b: version B' + assert_includes lines[2], 'ruby-c: version C' + end + end + describe '.render_graph' do it 'delegates to GraphRenderer and returns calculated png_path' do Dir.mktmpdir do |dir| From 0b7b37b0bdaa72a4c1ff92c283161ebc2af104b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 18 Nov 2025 16:26:57 +0000 Subject: [PATCH 7/8] Move base_name and other_names extraction into build_output_text --- lib/benchmark_runner.rb | 4 +++- run_benchmarks.rb | 3 +-- test/benchmark_runner_test.rb | 28 +++++++--------------------- 3 files changed, 11 insertions(+), 24 deletions(-) diff --git a/lib/benchmark_runner.rb b/lib/benchmark_runner.rb index d8df7401..8d5732c2 100644 --- a/lib/benchmark_runner.rb +++ b/lib/benchmark_runner.rb @@ -48,7 +48,9 @@ def write_csv(output_path, ruby_descriptions, table) end # Build output text string with metadata, table, and legend - def build_output_text(ruby_descriptions, table, format, bench_failures, base_name, other_names) + def build_output_text(ruby_descriptions, table, format, bench_failures) + base_name, *other_names = ruby_descriptions.keys + output_str = +"" ruby_descriptions.each do |key, value| diff --git a/run_benchmarks.rb b/run_benchmarks.rb index 91b10ac3..11b9f4de 100755 --- a/run_benchmarks.rb +++ b/run_benchmarks.rb @@ -57,7 +57,6 @@ # Build results table all_names = args.executables.keys -base_name, *other_names = all_names builder = ResultsTableBuilder.new( executable_names: all_names, bench_data: bench_data, @@ -76,7 +75,7 @@ BenchmarkRunner.write_csv(output_path, ruby_descriptions, table) # Save the output in a text file that we can easily refer to -output_str = BenchmarkRunner.build_output_text(ruby_descriptions, table, format, bench_failures, base_name, other_names) +output_str = BenchmarkRunner.build_output_text(ruby_descriptions, table, format, bench_failures) out_txt_path = output_path + ".txt" File.open(out_txt_path, "w") { |f| f.write output_str } diff --git a/test/benchmark_runner_test.rb b/test/benchmark_runner_test.rb index 8c47509c..8a8485e1 100644 --- a/test/benchmark_runner_test.rb +++ b/test/benchmark_runner_test.rb @@ -377,11 +377,9 @@ ] format = ['%s', '%.1f', '%.1f', '%.1f', '%.1f'] bench_failures = {} - base_name = 'ruby-base' - other_names = ['ruby-yjit'] result = BenchmarkRunner.build_output_text( - ruby_descriptions, table, format, bench_failures, base_name, other_names + ruby_descriptions, table, format, bench_failures ) assert_includes result, 'ruby-base: ruby 3.3.0' @@ -399,11 +397,9 @@ ] format = ['%s', '%.1f', '%.1f'] bench_failures = {} - base_name = 'ruby' - other_names = [] result = BenchmarkRunner.build_output_text( - ruby_descriptions, table, format, bench_failures, base_name, other_names + ruby_descriptions, table, format, bench_failures ) # Should contain table headers @@ -419,11 +415,9 @@ table = [['bench', 'ruby (ms)'], ['fib', '100.0']] format = ['%s', '%.1f'] bench_failures = {} - base_name = 'ruby' - other_names = [] result = BenchmarkRunner.build_output_text( - ruby_descriptions, table, format, bench_failures, base_name, other_names + ruby_descriptions, table, format, bench_failures ) refute_includes result, 'Legend:' @@ -438,11 +432,9 @@ table = [['bench', 'ruby (ms)', 'ruby-yjit (ms)', 'ruby-rjit (ms)']] format = ['%s', '%.1f', '%.1f', '%.1f'] bench_failures = {} - base_name = 'ruby' - other_names = ['ruby-yjit', 'ruby-rjit'] result = BenchmarkRunner.build_output_text( - ruby_descriptions, table, format, bench_failures, base_name, other_names + ruby_descriptions, table, format, bench_failures ) assert_includes result, 'ruby-yjit 1st itr' @@ -458,11 +450,9 @@ bench_failures = { 'ruby' => { 'failed_bench' => 'error message' } } - base_name = 'ruby' - other_names = [] result = BenchmarkRunner.build_output_text( - ruby_descriptions, table, format, bench_failures, base_name, other_names + ruby_descriptions, table, format, bench_failures ) # TableFormatter handles displaying failures, just verify it's called @@ -475,11 +465,9 @@ table = [['bench']] format = ['%s'] bench_failures = {} - base_name = 'ruby' - other_names = [] result = BenchmarkRunner.build_output_text( - ruby_descriptions, table, format, bench_failures, base_name, other_names + ruby_descriptions, table, format, bench_failures ) assert_kind_of String, result @@ -495,11 +483,9 @@ table = [['bench']] format = ['%s'] bench_failures = {} - base_name = 'ruby-a' - other_names = [] result = BenchmarkRunner.build_output_text( - ruby_descriptions, table, format, bench_failures, base_name, other_names + ruby_descriptions, table, format, bench_failures ) lines = result.lines From 5b3a1e13ded7ef1b2ec089c14dfabdf188a0785b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 18 Nov 2025 16:30:18 +0000 Subject: [PATCH 8/8] Remove unnecessary local variable --- run_benchmarks.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/run_benchmarks.rb b/run_benchmarks.rb index 11b9f4de..b78e4402 100755 --- a/run_benchmarks.rb +++ b/run_benchmarks.rb @@ -56,9 +56,8 @@ puts # Build results table -all_names = args.executables.keys builder = ResultsTableBuilder.new( - executable_names: all_names, + executable_names: ruby_descriptions.keys, bench_data: bench_data, include_rss: args.rss )