Fix TestRunFinished.success message to return true when the run has passed (#1606)

* Add a reproducible example for #1595

* Make sure module is valid before extending the world with it

* Update CHANGELOG.md

* Simplify world.feature

* Extend the world only if modules.any?

* Add reproducible example

* Add status of the run when emitting TestRunFinished

* Fix TestRunFinished.success message in the message builder

* Update CHANGELOG.md

* Update example to remove failing step actually not failing

* Add newline at the end of the feature file

* Make Runtime#failure? a real public method

Author: aurelien-reeves

CHANGELOG.md

@@ -16,6 +16,11 @@ Please visit [cucumber/CONTRIBUTING.md](https://github.com/cucumber/cucumber/blo
 
 ### Fixed
 
+- Fix TestRunFinished success property in html formatter and all formatters
+  based on the messages: it now returns true if the run has passed
+  ([PR#1606](https://github.com/cucumber/cucumber-ruby/pull/1606)
+   [Issue#1604](https://github.com/cucumber/cucumber-ruby/issues/1604))
+
 - Fix usage of namespaced modules across multiple scenarios
   ([PR#1603](https://github.com/cucumber/cucumber-ruby/pull/1603)
    [Issue#1595](https://github.com/cucumber/cucumber-ruby/issues/1595))

features/docs/formatters/message.feature

@@ -43,3 +43,34 @@ Feature: Message output formatter
       testRunFinished
       """
 
+  Scenario: it sets "testRunFinished"."success" to false if something failed
+    Given a file named "features/steps.rb" with:
+      """
+      Given('a passed step') {}
+      Given('a failed step') { fail }
+      """
+    When I run `cucumber features/my_feature.feature --format message`
+    Then output should be valid NDJSON
+    And the output should contain:
+      """
+      {"testRunFinished":{"success":false
+      """
+
+  Scenario: it sets "testRunFinished"."success" to true if nothing failed
+    Given a file named "features/my_feature.feature" with:
+      """
+      Feature: Some feature
+
+        Scenario Outline: a scenario
+          Given a passed step
+      """
+    And a file named "features/steps.rb" with:
+      """
+      Given('a passed step') {}
+      """
+    When I run `cucumber features/my_feature.feature --format message`
+    Then output should be valid NDJSON
+    And the output should contain:
+      """
+      {"testRunFinished":{"success":true
+      """

lib/cucumber/events/test_run_finished.rb

@@ -5,7 +5,8 @@
 module Cucumber
   module Events
     # Event fired after all test cases have finished executing
-    class TestRunFinished < Core::Event.new
+    class TestRunFinished < Core::Event.new(:success)
+      attr_reader :success
     end
   end
 end

lib/cucumber/formatter/message_builder.rb

@@ -226,10 +226,11 @@ def on_test_case_finished(event)
         output_envelope(message)
       end
 
-      def on_test_run_finished(*)
+      def on_test_run_finished(event)
         message = Cucumber::Messages::Envelope.new(
           test_run_finished: Cucumber::Messages::TestRunFinished.new(
-            timestamp: time_to_timestamp(Time.now)
+            timestamp: time_to_timestamp(Time.now),
+            success: event.success
           )
         )
 

lib/cucumber/runtime.rb

@@ -79,7 +79,7 @@ def run!
 
       receiver = Test::Runner.new(@configuration.event_bus)
       compile features, receiver, filters, @configuration.event_bus
-      @configuration.notify :test_run_finished
+      @configuration.notify :test_run_finished, !failure?
 
       fire_after_all_hook unless dry_run?
     end
@@ -110,6 +110,14 @@ def doc_string(string_without_triple_quotes, content_type = '', _line_offset = 0
       Core::Test::DocString.new(string_without_triple_quotes, content_type)
     end
 
+    def failure?
+      if @configuration.wip?
+        summary_report.test_cases.total_passed > 0
+      else
+        !summary_report.ok?(@configuration.strict)
+      end
+    end
+
     private
 
     def fire_install_plugin_hook # :nodoc:
@@ -226,15 +234,6 @@ def accept_options?(factory)
       factory.instance_method(:initialize).arity > 1
     end
 
-    def failure?
-      if @configuration.wip?
-        summary_report.test_cases.total_passed > 0
-      else
-        !summary_report.ok?(@configuration.strict)
-      end
-    end
-    public :failure?
-
     require 'cucumber/core/test/filters'
     def filters # rubocop:disable Metrics/AbcSize
       tag_expressions = @configuration.tag_expressions