From 1f250163f608db37a71e194421666faa5a1d7ff3 Mon Sep 17 00:00:00 2001
From: Iris Faraway <iris.faraway@gmail.com>
Date: Mon, 26 Apr 2021 16:58:30 +0100
Subject: [PATCH] Stop Quke from returning 0 exit status despite passing tests
 (#112)

https://eaflood.atlassian.net/browse/RUBY-1373

The default behaviour here is for Cucumber to exit as soon as a single test fails. We prefer to run the full suite each time, so these errors were being rescued.

However, this meant that if a test did fail, the exit status of the job would still be 0, which isn't what we want for our CI.

This PR changes the behaviour of Quke to record if a test fails, allow the test suite to proceed, and then throw the error at the end instead.

Cucumber will also call a SystemExit when the suite has finished running, or if there is some other problem with the suite. We don't want to consider the successful runs as errors or the suite will be considered "failed" every single time.
---
 lib/quke.rb             |  7 ++++++-
 lib/quke/cuke_runner.rb | 19 +++++++++++++------
 spec/quke/quke_spec.rb  | 15 +++++++++++++++
 3 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/lib/quke.rb b/lib/quke.rb
index b65d639..91a8686 100644
--- a/lib/quke.rb
+++ b/lib/quke.rb
@@ -15,6 +15,8 @@
 
 module Quke #:nodoc:
 
+  class QukeError < StandardError; end
+
   # The main Quke class. It is not intended to be instantiated and instead
   # just need to call its +execute+ method.
   class Quke
@@ -28,7 +30,10 @@ class << self
     # The entry point for Quke, it is the one call made by +exe/quke+.
     def self.execute(args = [])
       cuke = CukeRunner.new(args)
-      cuke.run
+      errors = cuke.run
+      return if errors.empty?
+
+      raise QukeError.new, "Number of failures or errors: #{errors.count}"
     end
 
   end
diff --git a/lib/quke/cuke_runner.rb b/lib/quke/cuke_runner.rb
index d770ba2..9a39132 100644
--- a/lib/quke/cuke_runner.rb
+++ b/lib/quke/cuke_runner.rb
@@ -35,12 +35,19 @@ def initialize(passed_in_args = [])
     # Executes Cucumber passing in the arguments array, which was set when the
     # instance of CukeRunner was initialized.
     def run
-      Cucumber::Cli::Main.new(@args).execute!
-    rescue SystemExit
-      # Cucumber calls @kernel.exit() killing your script unless you rescue
-      # If any tests fail cucumber will exit with an error code however this
-      # is expected and normal behaviour. We capture the exit to prevent it
-      # bubbling up to our app and closing it.
+      errors = []
+
+      begin
+        Cucumber::Cli::Main.new(@args).execute!
+      rescue SystemExit => e
+        # Cucumber calls @kernel.exit() whenever a test fails, or when the test
+        # suite has finished running. We prefer to run the full test suite every
+        # time, and then fail at the end. However if the SystemExit is a
+        # successful one, we don't want to log it as an error.
+        errors << e unless e.success?
+      end
+
+      errors
     end
 
   end
diff --git a/spec/quke/quke_spec.rb b/spec/quke/quke_spec.rb
index 1f5674d..52dce6a 100644
--- a/spec/quke/quke_spec.rb
+++ b/spec/quke/quke_spec.rb
@@ -60,5 +60,20 @@
       end
     end
 
+    context "when one of the tests throws a SystemError with status 1" do
+      before { expect(Cucumber::Cli::Main).to receive(:new).and_raise(SystemExit, 1) }
+
+      it "holds onto the errors and raises them at the end" do
+        expect { Quke::Quke.execute }.to raise_error(Quke::QukeError, "Number of failures or errors: 1")
+      end
+    end
+
+    context "when one of the tests throws a SystemError with status 0" do
+      before { expect(Cucumber::Cli::Main).to receive(:new).and_raise(SystemExit, 0) }
+
+      it "does not raise an error" do
+        expect { Quke::Quke.execute }.not_to raise_error
+      end
+    end
   end
 end