From 53c8804a277b02021286f3f05bf77f2c534b73bd Mon Sep 17 00:00:00 2001
From: Dominik <dominik.schoeler@makandra.de>
Date: Wed, 19 Feb 2025 17:37:32 +0100
Subject: [PATCH] Fix output of ParallelTests::Cucumber::FailuresLogger (fixes
 #979) (#990)

---
 CHANGELOG.md                                  |  2 +
 .../cucumber/failures_logger.rb               | 23 +++++++---
 .../cucumber/failure_logger_spec.rb           | 45 ++++++-------------
 3 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9c8eaa1f..38293d71 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,8 @@
 
 ### Fixed
 
+- Fix output of Cucumber failures logger. Previously, an event handler inherited from `Cucumber::Formatter::Rerun` would improperly join failures (e.g. `feature/one.feature:1feature/two.feature:1`). Now failures are separated with a single space.
+
 ## 4.9.0 - 2025-01-09
 
 ### Fixed
diff --git a/lib/parallel_tests/cucumber/failures_logger.rb b/lib/parallel_tests/cucumber/failures_logger.rb
index 911ed1d4..0c3391c5 100644
--- a/lib/parallel_tests/cucumber/failures_logger.rb
+++ b/lib/parallel_tests/cucumber/failures_logger.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 require 'cucumber/formatter/rerun'
 require 'parallel_tests/gherkin/io'
+require 'cucumber/events'
 
 module ParallelTests
   module Cucumber
@@ -9,15 +10,23 @@ class FailuresLogger < ::Cucumber::Formatter::Rerun
 
       def initialize(config)
         super
+
         @io = prepare_io(config.out_stream)
-      end
 
-      def done
-        return if @failures.empty?
-        lock_output do
-          @failures.each do |file, lines|
-            lines.each do |line|
-              @io.print "#{file}:#{line} "
+        # Remove handler inherited from Cucumber::Formatter::Rerun that does not
+        # properly join file failures
+        handlers = config.event_bus.instance_variable_get(:@handlers)
+        handlers[::Cucumber::Events::TestRunFinished.to_s].pop
+
+        # Add our own handler
+        config.on_event :test_run_finished do
+          return if @failures.empty?
+
+          lock_output do
+            @failures.each do |file, lines|
+              lines.each do |line|
+                @io.print "#{file}:#{line} "
+              end
             end
           end
         end
diff --git a/spec/parallel_tests/cucumber/failure_logger_spec.rb b/spec/parallel_tests/cucumber/failure_logger_spec.rb
index bded4218..fd96b7d5 100644
--- a/spec/parallel_tests/cucumber/failure_logger_spec.rb
+++ b/spec/parallel_tests/cucumber/failure_logger_spec.rb
@@ -2,44 +2,25 @@
 require 'spec_helper'
 require 'parallel_tests/gherkin/io'
 require 'parallel_tests/cucumber/failures_logger'
+require 'cucumber/configuration'
 
 describe ParallelTests::Cucumber::FailuresLogger do
-  before do
-    @output = OutputLogger.new([])
-    allow(@output).to receive(:write)
+  let(:parallel_cucumber_failures) { StringIO.new }
+  let(:config) { Cucumber::Configuration.new(out_stream: parallel_cucumber_failures) }
 
-    config = double('config', out_stream: @output, on_event: :test_run_finished)
+  let(:logger1) { ParallelTests::Cucumber::FailuresLogger.new(config) }
+  let(:logger2) { ParallelTests::Cucumber::FailuresLogger.new(config) }
 
-    @logger1 = ParallelTests::Cucumber::FailuresLogger.new(config)
-    @logger2 = ParallelTests::Cucumber::FailuresLogger.new(config)
-    @logger3 = ParallelTests::Cucumber::FailuresLogger.new(config)
+  it "should produce a list of failing scenarios" do
+    feature1 = double('feature', file: "feature/one.feature")
+    feature2 = double('feature', file: "feature/two.feature")
 
-    @feature1 = double('feature', file: "feature/path/to/feature1.feature")
-    @feature2 = double('feature', file: "feature/path/to/feature2.feature")
-    @feature3 = double('feature', file: "feature/path/to/feature3.feature")
+    logger1.instance_variable_set("@failures", { feature1.file => [1, 3] })
+    logger2.instance_variable_set("@failures", { feature2.file => [2, 4] })
 
-    @logger1.instance_variable_set("@failures", { @feature1.file => [1, 2, 3] })
-    @logger2.instance_variable_set("@failures", { @feature2.file => [2, 4, 6] })
-    @logger3.instance_variable_set("@failures", { @feature3.file => [3, 6, 9] })
-  end
-
-  it "should produce a list of lines for failing scenarios" do
-    @logger1.done
-    @logger2.done
-    @logger3.done
-
-    output_file_contents = @output.output.join("\n").concat("\n")
+    config.event_bus.broadcast(Cucumber::Events::TestRunFinished.new)
+    parallel_cucumber_failures.rewind
 
-    expect(output_file_contents).to eq <<~END
-      feature/path/to/feature1.feature:1\s
-      feature/path/to/feature1.feature:2\s
-      feature/path/to/feature1.feature:3\s
-      feature/path/to/feature2.feature:2\s
-      feature/path/to/feature2.feature:4\s
-      feature/path/to/feature2.feature:6\s
-      feature/path/to/feature3.feature:3\s
-      feature/path/to/feature3.feature:6\s
-      feature/path/to/feature3.feature:9\s
-    END
+    expect(parallel_cucumber_failures.read).to eq 'feature/one.feature:1 feature/one.feature:3 feature/two.feature:2 feature/two.feature:4 '
   end
 end