From eec7200a2cc5fc11a525d0e5156674c7680d36ff Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Wed, 12 Oct 2022 15:44:25 +0100
Subject: [PATCH] Improve test runner assertions - failure vs error

Make more of a distinction between test failures and errors. Explicitly
assert for what we are expecting and flunk with an appropriate error
message if that's not what happened.
---
 .../stubbing_error_backtrace_test.rb           |  8 ++++----
 test/acceptance/stubbing_frozen_object_test.rb | 18 ++++++------------
 .../stubbing_method_unnecessarily_test.rb      |  2 +-
 test/acceptance/stubbing_nil_test.rb           |  4 ++--
 ...ng_non_existent_any_instance_method_test.rb |  2 +-
 .../stubbing_non_existent_class_method_test.rb |  2 +-
 ...ubbing_non_existent_instance_method_test.rb |  2 +-
 ...bing_non_public_any_instance_method_test.rb |  4 ++--
 .../stubbing_non_public_class_method_test.rb   |  4 ++--
 ...stubbing_non_public_instance_method_test.rb |  4 ++--
 .../stubbing_on_non_mock_object_test.rb        |  2 +-
 test/integration/shared_tests.rb               |  2 +-
 test/test_runner.rb                            |  9 +++++++--
 13 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/test/acceptance/stubbing_error_backtrace_test.rb b/test/acceptance/stubbing_error_backtrace_test.rb
index dd07d5be9..05c5bb03e 100644
--- a/test/acceptance/stubbing_error_backtrace_test.rb
+++ b/test/acceptance/stubbing_error_backtrace_test.rb
@@ -20,7 +20,7 @@ def test_should_display_backtrace_indicating_line_number_where_attempt_to_stub_n
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; object.stubs(:non_existent_method)
     end
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
 
@@ -34,7 +34,7 @@ def non_public_method; end
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; object.stubs(:non_public_method)
     end
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
 
@@ -45,7 +45,7 @@ def test_should_display_backtrace_indicating_line_number_where_attempt_to_stub_m
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; object.stubs(:any_method)
     end
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
 
@@ -56,7 +56,7 @@ def test_should_display_backtrace_indicating_line_number_where_method_was_unnece
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; object.stubs(:unused_method)
     end
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
   # rubocop:enable Style/Semicolon
diff --git a/test/acceptance/stubbing_frozen_object_test.rb b/test/acceptance/stubbing_frozen_object_test.rb
index 6654295a7..aa8b0c830 100644
--- a/test/acceptance/stubbing_frozen_object_test.rb
+++ b/test/acceptance/stubbing_frozen_object_test.rb
@@ -20,8 +20,7 @@ def test_should_fail_fast_if_attempting_to_stub_method_on_frozen_object
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; object.stubs(:stubbed_method)
     end
-    assert_failed(test_result)
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
 
@@ -32,8 +31,7 @@ def test_should_fail_fast_if_attempting_to_expect_method_on_frozen_object
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; object.expects(:stubbed_method)
     end
-    assert_failed(test_result)
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
 
@@ -44,8 +42,7 @@ def test_should_fail_fast_if_attempting_to_stub_method_on_frozen_class
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; klass.stubs(:stubbed_method)
     end
-    assert_failed(test_result)
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
 
@@ -56,8 +53,7 @@ def test_should_fail_fast_if_attempting_to_expect_method_on_frozen_class
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; klass.expects(:stubbed_method)
     end
-    assert_failed(test_result)
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
 
@@ -68,8 +64,7 @@ def test_should_fail_fast_if_attempting_to_stub_method_on_any_instance_of_frozen
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; klass.any_instance.stubs(:stubbed_method)
     end
-    assert_failed(test_result)
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
 
@@ -80,8 +75,7 @@ def test_should_fail_fast_if_attempting_to_expect_method_on_any_instance_of_froz
     test_result = run_as_test do
       execution_point = ExecutionPoint.current; klass.any_instance.expects(:stubbed_method)
     end
-    assert_failed(test_result)
-    assert_equal 1, test_result.error_count
+    assert_errored(test_result)
     assert_equal execution_point, ExecutionPoint.new(test_result.errors[0].exception.backtrace)
   end
   # rubocop:enable Style/Semicolon
diff --git a/test/acceptance/stubbing_method_unnecessarily_test.rb b/test/acceptance/stubbing_method_unnecessarily_test.rb
index 12ef4fa8f..39988c00e 100644
--- a/test/acceptance/stubbing_method_unnecessarily_test.rb
+++ b/test/acceptance/stubbing_method_unnecessarily_test.rb
@@ -38,7 +38,7 @@ def test_should_prevent_stubbing_method_unnecessarily
       mock = mock('mock')
       mock.stubs(:public_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?('Mocha::StubbingError: stubbing method unnecessarily: #<Mock:mock>.public_method(any_parameters)')
   end
 
diff --git a/test/acceptance/stubbing_nil_test.rb b/test/acceptance/stubbing_nil_test.rb
index 361083d2e..e7777fb02 100644
--- a/test/acceptance/stubbing_nil_test.rb
+++ b/test/acceptance/stubbing_nil_test.rb
@@ -36,7 +36,7 @@ def test_should_prevent_stubbing_method_on_nil
       test_result = run_as_test do
         nil.stubs(:stubbed_method)
       end
-      assert_failed(test_result)
+      assert_errored(test_result)
       assert test_result.error_messages.include?('Mocha::StubbingError: stubbing method on nil: nil.stubbed_method')
     end
 
@@ -44,7 +44,7 @@ def test_should_default_to_prevent_stubbing_method_on_non_mock_object
       test_result = run_as_test do
         nil.stubs(:stubbed_method)
       end
-      assert_failed(test_result)
+      assert_errored(test_result)
       assert test_result.error_messages.include?('Mocha::StubbingError: stubbing method on nil: nil.stubbed_method')
     end
 
diff --git a/test/acceptance/stubbing_non_existent_any_instance_method_test.rb b/test/acceptance/stubbing_non_existent_any_instance_method_test.rb
index eb102b09f..8097a4f81 100644
--- a/test/acceptance/stubbing_non_existent_any_instance_method_test.rb
+++ b/test/acceptance/stubbing_non_existent_any_instance_method_test.rb
@@ -38,7 +38,7 @@ def test_should_prevent_stubbing_non_existent_any_instance_method
     test_result = run_as_test do
       klass.any_instance.stubs(:non_existent_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing non-existent method: #{klass.any_instance.mocha_inspect}.non_existent_method")
   end
 
diff --git a/test/acceptance/stubbing_non_existent_class_method_test.rb b/test/acceptance/stubbing_non_existent_class_method_test.rb
index 053d6db3d..eba960d3c 100644
--- a/test/acceptance/stubbing_non_existent_class_method_test.rb
+++ b/test/acceptance/stubbing_non_existent_class_method_test.rb
@@ -38,7 +38,7 @@ def test_should_prevent_stubbing_non_existent_class_method
     test_result = run_as_test do
       klass.stubs(:non_existent_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing non-existent method: #{klass.mocha_inspect}.non_existent_method")
   end
 
diff --git a/test/acceptance/stubbing_non_existent_instance_method_test.rb b/test/acceptance/stubbing_non_existent_instance_method_test.rb
index 4e0c3aab2..747ab7bc4 100644
--- a/test/acceptance/stubbing_non_existent_instance_method_test.rb
+++ b/test/acceptance/stubbing_non_existent_instance_method_test.rb
@@ -38,7 +38,7 @@ def test_should_prevent_stubbing_non_existent_instance_method
     test_result = run_as_test do
       instance.stubs(:non_existent_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing non-existent method: #{instance.mocha_inspect}.non_existent_method")
   end
 
diff --git a/test/acceptance/stubbing_non_public_any_instance_method_test.rb b/test/acceptance/stubbing_non_public_any_instance_method_test.rb
index 4f4cded4b..9fa8e9fea 100644
--- a/test/acceptance/stubbing_non_public_any_instance_method_test.rb
+++ b/test/acceptance/stubbing_non_public_any_instance_method_test.rb
@@ -73,7 +73,7 @@ def private_method; end
     test_result = run_as_test do
       klass.any_instance.stubs(:private_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing non-public method: #{klass.any_instance.mocha_inspect}.private_method")
   end
 
@@ -86,7 +86,7 @@ def protected_method; end
     test_result = run_as_test do
       klass.any_instance.stubs(:protected_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing non-public method: #{klass.any_instance.mocha_inspect}.protected_method")
   end
 
diff --git a/test/acceptance/stubbing_non_public_class_method_test.rb b/test/acceptance/stubbing_non_public_class_method_test.rb
index 550a7496c..23b5114a3 100644
--- a/test/acceptance/stubbing_non_public_class_method_test.rb
+++ b/test/acceptance/stubbing_non_public_class_method_test.rb
@@ -84,7 +84,7 @@ def private_method; end
     test_result = run_as_test do
       klass.stubs(:private_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing non-public method: #{klass.mocha_inspect}.private_method")
   end
 
@@ -99,7 +99,7 @@ def protected_method; end
     test_result = run_as_test do
       klass.stubs(:protected_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing non-public method: #{klass.mocha_inspect}.protected_method")
   end
 
diff --git a/test/acceptance/stubbing_non_public_instance_method_test.rb b/test/acceptance/stubbing_non_public_instance_method_test.rb
index 0c645d60b..d70f7aaaa 100644
--- a/test/acceptance/stubbing_non_public_instance_method_test.rb
+++ b/test/acceptance/stubbing_non_public_instance_method_test.rb
@@ -73,7 +73,7 @@ def private_method; end
     test_result = run_as_test do
       instance.stubs(:private_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing non-public method: #{instance.mocha_inspect}.private_method")
   end
 
@@ -86,7 +86,7 @@ def protected_method; end
     test_result = run_as_test do
       instance.stubs(:protected_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing non-public method: #{instance.mocha_inspect}.protected_method")
   end
 
diff --git a/test/acceptance/stubbing_on_non_mock_object_test.rb b/test/acceptance/stubbing_on_non_mock_object_test.rb
index d1a08d2a7..58ffe8bd7 100644
--- a/test/acceptance/stubbing_on_non_mock_object_test.rb
+++ b/test/acceptance/stubbing_on_non_mock_object_test.rb
@@ -44,7 +44,7 @@ def existing_method; end
     test_result = run_as_test do
       non_mock_object.stubs(:existing_method)
     end
-    assert_failed(test_result)
+    assert_errored(test_result)
     assert test_result.error_messages.include?("Mocha::StubbingError: stubbing method on non-mock object: #{non_mock_object.mocha_inspect}.existing_method")
   end
 
diff --git a/test/integration/shared_tests.rb b/test/integration/shared_tests.rb
index d98ee3755..e5075960a 100644
--- a/test/integration/shared_tests.rb
+++ b/test/integration/shared_tests.rb
@@ -168,7 +168,7 @@ def test_real_object_expectation_does_not_leak_into_subsequent_test
         execution_point = ExecutionPoint.current; klass.foo
       }
     )
-    assert_failed(test_result)
+    assert_errored(test_result)
     exception = test_result.errors.first.exception
     assert_equal execution_point, ExecutionPoint.new(exception.backtrace)
     assert_match(/undefined method `foo'/, exception.message)
diff --git a/test/test_runner.rb b/test/test_runner.rb
index 51f7f1974..e3f6375a1 100644
--- a/test/test_runner.rb
+++ b/test/test_runner.rb
@@ -48,11 +48,16 @@ def run_as_tests(methods = {})
   # rubocop:enable Metrics/AbcSize
 
   def assert_passed(test_result)
+    flunk "Test errored unexpectedly with message: #{test_result.errors.map(&:exception)}" if test_result.error_count > 0
     flunk "Test failed unexpectedly with message: #{test_result.failures}" if test_result.failure_count > 0
-    flunk "Test failed unexpectedly with message: #{test_result.errors.map(&:exception)}" if test_result.error_count > 0
   end
 
   def assert_failed(test_result)
-    flunk 'Test passed unexpectedly' unless test_result.failure_count + test_result.error_count > 0
+    flunk "Test errored unexpectedly with message: #{test_result.errors.map(&:exception)}" if test_result.error_count > 0
+    flunk 'Test passed unexpectedly' unless test_result.failure_count > 0
+  end
+
+  def assert_errored(test_result)
+    flunk 'Test did not error as expected' unless test_result.error_count > 0
   end
 end