bugfix: HTML Formatter and its envelopes

CHANGELOG.md

@@ -10,6 +10,7 @@ Please visit [cucumber/CONTRIBUTING.md](https://github.com/cucumber/cucumber/blo
 
 ## [Unreleased]
 ### Fixed
+- Fixed issue with `html-formatter` where attachments and envelopes were causing the entire message pool to be blank ([#1891](https://github.com/cucumber/cucumber-ruby/pull/1891)) [luke-hill](https://github.com/luke-hill)
 - Show failed step error details in the summary formatter output
 - Fixed up JRuby examples which weren't running due to anglicisation issues (Pivoted to use English step definitions to help JRuby testing)
 - Fixed up Arabic example which had some incorrect logic for step definition matching (Due to RTL nature of the language)

lib/cucumber/events/attach_called.rb

@@ -5,7 +5,7 @@
 module Cucumber
   module Events
     # Fired when attach is called in a step definition
-    class AttachCalled < Core::Event.new(:src, :media_type, :filename, :streamed_file)
+    class AttachCalled < Core::Event.new(:src, :media_type, :filename)
       # The attachment body
       attr_reader :src
 
@@ -14,9 +14,6 @@ class AttachCalled < Core::Event.new(:src, :media_type, :filename, :streamed_fil
 
       # An optional filename
       attr_reader :filename
-
-      # Whether the file is streamed or not
-      attr_reader :streamed_file
     end
   end
 end

lib/cucumber/formatter/html.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
-require 'cucumber/formatter/io'
 require 'cucumber/html_formatter'
-require 'cucumber/formatter/message_builder'
+
+require_relative 'message_builder'
 
 module Cucumber
   module Formatter
@@ -14,9 +14,11 @@ def initialize(config)
         super(config)
       end
 
-      def output_envelope(envelope)
-        @repository.update(envelope)
+      def on_envelope(event)
+        super(event)
+        envelope = event.envelope
         @html_formatter.write_message(envelope)
+        # TODO: Move this conditional logic into the HTML formatter proper
         @html_formatter.write_post_message if envelope.test_run_finished
       end
     end

lib/cucumber/formatter/message.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
-require 'cucumber/formatter/io'
-require 'cucumber/query'
+require_relative 'io'
 
 module Cucumber
   module Formatter
@@ -11,14 +10,11 @@ class Message
 
       def initialize(config)
         @io = ensure_io(config.out_stream, config.error_stream)
-        @repository = Cucumber::Repository.new
-        @query = Cucumber::Query.new(@repository)
         config.on_event :envelope, &method(:output_envelope)
       end
 
       def output_envelope(event)
         envelope = event.envelope
-        @repository.update(envelope)
         @io.write(envelope.to_json)
         @io.write("\n")
       end

lib/cucumber/formatter/message_builder.rb

@@ -6,11 +6,14 @@
 require 'cucumber/formatter/backtrace_filter'
 require 'cucumber/query'
 
+require_relative 'message_handlers'
+
 module Cucumber
   module Formatter
     class MessageBuilder
       include Cucumber::Messages::Helpers::TimeConversion
       include Io
+      include MessageHandlers
       include Console
 
       def initialize(config)
@@ -53,7 +56,7 @@ def initialize(config)
       end
 
       def on_envelope(event)
-        @current_test_run_hook_started_id = event.envelope.test_run_hook_started.id if event.envelope.test_run_hook_started
+        store_current_test_run_hook_started_id(event)
       end
 
       def on_attach_called(event)
@@ -75,7 +78,9 @@ def on_attach_called(event)
             }
           end
 
-        if event.streamed_file
+        streamed_file = event.src.encoding == Encoding::BINARY
+
+        if streamed_file
           attachment_data[:content_encoding] = Cucumber::Messages::AttachmentContentEncoding::BASE64
           attachment_data[:body] = Base64.strict_encode64(event.src)
         else

lib/cucumber/formatter/message_handlers.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module Cucumber
+  module Formatter
+    # Common Message Handlers to be used across all message-based formatters
+    # Designed to work solely with events of type `Envelope`
+    module MessageHandlers
+      def store_current_test_run_hook_started_id(event)
+        @current_test_run_hook_started_id = event.envelope.test_run_hook_started.id if event.envelope.test_run_hook_started
+      end
+    end
+  end
+end

lib/cucumber/formatter/rerun.rb

@@ -1,8 +1,9 @@
 # frozen_string_literal: true
 
-require 'cucumber/formatter/io'
 require 'cucumber/query'
 
+require_relative 'io'
+
 module Cucumber
   module Formatter
     class Rerun

lib/cucumber/glue/proto_world.rb

@@ -93,12 +93,11 @@ def attach(file, media_type = nil, filename = nil)
         if File.file?(file)
           media_type = MiniMime.lookup_by_filename(file)&.content_type if media_type.nil?
           file = File.read(file, mode: 'rb')
-          streamed_file = true
         end
         # We pass in the concept of whether the file is streamed to ensure that the envelope encoding is correct
-        super(file, media_type, filename, streamed_file)
+        super(file, media_type, filename)
       rescue StandardError
-        super(file, media_type, filename, streamed_file)
+        super(file, media_type, filename)
       end
 
       # Mark the matched step as pending.
@@ -155,8 +154,8 @@ def add_modules!(world_modules, namespaced_world_modules)
             runtime.ask(question, timeout_seconds)
           end
 
-          define_method(:attach) do |file, media_type, filename, streamed_file|
-            runtime.attach(file, media_type, filename, streamed_file)
+          define_method(:attach) do |file, media_type, filename|
+            runtime.attach(file, media_type, filename)
           end
 
           # Prints the list of modules that are included in the World

lib/cucumber/runtime/user_interface.rb

@@ -42,8 +42,8 @@ def ask(question, timeout_seconds)
       # be a path to a file, or if it's an image it may also be a Base64 encoded image.
       # The embedded data may or may not be ignored, depending on what kind of formatter(s) are active.
       #
-      def attach(src, media_type, filename, streamed_file)
-        @configuration.notify(:attach_called, src, media_type, filename, streamed_file)
+      def attach(src, media_type, filename)
+        @configuration.notify(:attach_called, src, media_type, filename)
         self
       end
 

spec/cucumber/glue/step_definition_spec.rb

@@ -191,14 +191,14 @@ def step_match(text)
 
       describe '#log' do
         it 'calls "attach" with the correct media type' do
-          expect(user_interface).to receive(:attach).with('wasup', 'text/x.cucumber.log+plain', nil, nil)
+          expect(user_interface).to receive(:attach).with('wasup', 'text/x.cucumber.log+plain', nil)
 
           dsl.Given('Loud') { log 'wasup' }
           run_step 'Loud'
         end
 
         it 'calls `to_s` if the message is not a String' do
-          expect(user_interface).to receive(:attach).with('["Not", 1, "string"]', 'text/x.cucumber.log+plain', nil, nil)
+          expect(user_interface).to receive(:attach).with('["Not", 1, "string"]', 'text/x.cucumber.log+plain', nil)
 
           dsl.Given('Loud') { log ['Not', 1, 'string'] }
           run_step 'Loud'