Move logic to build the output path to BenchmarkRunner

Author: rafaelfranca

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

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"

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|

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