diff --git a/CHANGELOG.md b/CHANGELOG.md index 66d0416ae..134afbde6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ Please visit [cucumber/CONTRIBUTING.md](https://github.com/cucumber/cucumber/blo ### Changed - Heavy refactor to the internals for message building (Used in formatters - should be no noticeable change) ([#1853](https://github.com/cucumber/cucumber-ruby/pull/1853) [luke-hill](https://github.com/luke-hill)) +- Simplify attachment handling in the `MessageBuilder` and `#attach` method + +([#1853]( +### Fixed +- When someone `#attach`s a hashified output (Instead of JSON), call `#to_json` before attaching as a stringified JSON response ## [11.0.0] - 2026-04-14 ### Added diff --git a/features/docs/fixtures/cucumber.jpeg b/features/docs/fixtures/cucumber.jpeg new file mode 100644 index 000000000..e833d6c77 Binary files /dev/null and b/features/docs/fixtures/cucumber.jpeg differ diff --git a/features/docs/writing_support_code/attachments.feature b/features/docs/writing_support_code/attachments.feature index d868abc19..afe39eae8 100644 --- a/features/docs/writing_support_code/attachments.feature +++ b/features/docs/writing_support_code/attachments.feature @@ -20,18 +20,15 @@ Feature: Attachments Scenario: Given I attach a screenshot without a media type """ - And a file named "features/screenshot.png" with: - """ - foo - """ + And an image named "cucumber.jpeg" And a file named "features/step_definitions/attaching_screenshot_steps.rb" with: """ Given('I attach a screenshot with a media type') do - attach('features/screenshot.png', 'image/png') + attach('features/cucumber.jpeg', 'image/jpeg') end Given('I attach a screenshot without a media type') do - attach('features/screenshot.png') + attach('features/cucumber.jpeg') end """ @@ -39,18 +36,18 @@ Feature: Attachments When I run `cucumber --format message features/attaching_screenshot_with_mediatype.feature` Then output should be valid NDJSON And the output should contain NDJSON with key "attachment" - And the output should contain NDJSON "attachment" message with key "body" and value "Zm9v" - And the output should contain NDJSON "attachment" message with key "mediaType" and value "image/png" + And the output should contain NDJSON "attachment" message with key "body" and an encoded value + And the output should contain NDJSON "attachment" message with key "mediaType" and value "image/jpeg" Scenario: Media type is inferred from the given file When I run `cucumber --format message features/attaching_screenshot_without_mediatype.feature` Then output should be valid NDJSON And the output should contain NDJSON with key "attachment" - And the output should contain NDJSON "attachment" message with key "body" and value "Zm9v" - And the output should contain NDJSON "attachment" message with key "mediaType" and value "image/png" + And the output should contain NDJSON "attachment" message with key "body" and an encoded value + And the output should contain NDJSON "attachment" message with key "mediaType" and value "image/jpeg" Scenario: With json formatter, files can be attached given their path When I run `cucumber --format json features/attaching_screenshot_with_mediatype.feature` Then the output should contain "embeddings\":" - And the output should contain "\"mime_type\": \"image/png\"," - And the output should contain "\"data\": \"Zm9v\"" + And the output should contain "\"mime_type\": \"image/jpeg\"," + And the output should contain "\"data\":" diff --git a/features/lib/step_definitions/cucumber_steps.rb b/features/lib/step_definitions/cucumber_steps.rb index dc7c830d3..9b33e62a3 100644 --- a/features/lib/step_definitions/cucumber_steps.rb +++ b/features/lib/step_definitions/cucumber_steps.rb @@ -15,7 +15,7 @@ ' @io = config.out_stream', ' end', '', - ' def attach(src, media_type, _filename)', + ' def attach(src, media_type, _filename, _streamed_file)', ' @io.puts(src)', ' end', 'end' diff --git a/features/lib/step_definitions/filesystem_steps.rb b/features/lib/step_definitions/filesystem_steps.rb index ce719214d..a85cb8b82 100644 --- a/features/lib/step_definitions/filesystem_steps.rb +++ b/features/lib/step_definitions/filesystem_steps.rb @@ -8,6 +8,10 @@ write_file(path, content) end +Given('an image named {string}') do |name| + copy_image_named(name) +end + Given('an empty file named {string}') do |path| write_file(path, '') end diff --git a/features/lib/step_definitions/message_steps.rb b/features/lib/step_definitions/message_steps.rb index 4996bf88b..40dec1927 100644 --- a/features/lib/step_definitions/message_steps.rb +++ b/features/lib/step_definitions/message_steps.rb @@ -18,6 +18,12 @@ expect(command_line.stdout).to match(/"#{key}": ?"#{value}"/) end +Then('the output should contain NDJSON {string} message with key {string} and an encoded value') do |message_name, key| + message_contents = command_line.stdout(format: :messages).detect { |msg| msg.keys == [message_name] }[message_name] + + expect(message_contents[key].length).to be > 50 +end + Then('the output should contain NDJSON {string} message with key {string} and value {string}') do |message_name, key, value| message_contents = command_line.stdout(format: :messages).detect { |msg| msg.keys == [message_name] }[message_name] diff --git a/features/lib/support/filesystem.rb b/features/lib/support/filesystem.rb index fa99b5a83..d12d13d80 100644 --- a/features/lib/support/filesystem.rb +++ b/features/lib/support/filesystem.rb @@ -4,3 +4,8 @@ def write_file(path, content) FileUtils.mkdir_p(File.dirname(path)) File.open(path, 'w') { |file| file.write(content) } end + +def copy_image_named(name) + fixture_dir = File.expand_path('../../features/docs/fixtures') + FileUtils.cp("#{fixture_dir}/#{name}", "#{Dir.pwd}/features/#{name}") +end diff --git a/lib/cucumber/formatter/console.rb b/lib/cucumber/formatter/console.rb index dfe7688ae..194698f56 100644 --- a/lib/cucumber/formatter/console.rb +++ b/lib/cucumber/formatter/console.rb @@ -169,7 +169,7 @@ def do_print_passing_wip(passed_messages) end end - def attach(src, media_type, filename) + def attach(src, media_type, filename, _streamed_file) return unless media_type == 'text/x.cucumber.log+plain' return unless @io diff --git a/lib/cucumber/formatter/json.rb b/lib/cucumber/formatter/json.rb index 8fc8471a4..401d762cb 100644 --- a/lib/cucumber/formatter/json.rb +++ b/lib/cucumber/formatter/json.rb @@ -85,7 +85,7 @@ def on_test_run_finished(_event) @io.write(JSON.pretty_generate(@feature_hashes)) end - def attach(src, mime_type, _filename) + def attach(src, mime_type, _filename, _streamed_file) if mime_type == 'text/x.cucumber.log+plain' test_step_output << src return diff --git a/lib/cucumber/formatter/message_builder.rb b/lib/cucumber/formatter/message_builder.rb index 86a0e9204..92f5335a8 100644 --- a/lib/cucumber/formatter/message_builder.rb +++ b/lib/cucumber/formatter/message_builder.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true require 'base64' -require 'cucumber/formatter/backtrace_filter' +require 'json' +require 'cucumber/formatter/backtrace_filter' require 'cucumber/query' module Cucumber @@ -55,7 +56,7 @@ def initialize(config) config.on_event :undefined_parameter_type, &method(:on_undefined_parameter_type) end - def attach(src, media_type, filename) + def attach(src, media_type, filename, streamed_file) attachment_data = { test_step_id: @current_test_step_id, test_case_started_id: @current_test_case_started_id, @@ -64,13 +65,12 @@ def attach(src, media_type, filename) timestamp: time_to_timestamp(Time.now) } - if media_type&.start_with?('text/') - attachment_data[:content_encoding] = Cucumber::Messages::AttachmentContentEncoding::IDENTITY - attachment_data[:body] = src - else - body = src.respond_to?(:read) ? src.read : src + if streamed_file attachment_data[:content_encoding] = Cucumber::Messages::AttachmentContentEncoding::BASE64 - attachment_data[:body] = Base64.strict_encode64(body) + attachment_data[:body] = Base64.strict_encode64(src) + else + attachment_data[:content_encoding] = Cucumber::Messages::AttachmentContentEncoding::IDENTITY + attachment_data[:body] = src.is_a?(Hash) ? src.to_json : src end message = Cucumber::Messages::Envelope.new(attachment: Cucumber::Messages::Attachment.new(**attachment_data)) diff --git a/lib/cucumber/formatter/pretty.rb b/lib/cucumber/formatter/pretty.rb index 97ba8d796..7f4549703 100644 --- a/lib/cucumber/formatter/pretty.rb +++ b/lib/cucumber/formatter/pretty.rb @@ -140,7 +140,7 @@ def on_test_run_finished(_event) print_summary end - def attach(src, media_type, filename) + def attach(src, media_type, filename, _streamed_file) return unless media_type == 'text/x.cucumber.log+plain' if filename diff --git a/lib/cucumber/glue/proto_world.rb b/lib/cucumber/glue/proto_world.rb index 83452aa8d..a797d34b8 100644 --- a/lib/cucumber/glue/proto_world.rb +++ b/lib/cucumber/glue/proto_world.rb @@ -93,10 +93,12 @@ 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 - super + # 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) rescue StandardError - super + super(file, media_type, filename, streamed_file) end # Mark the matched step as pending. @@ -153,8 +155,8 @@ def add_modules!(world_modules, namespaced_world_modules) runtime.ask(question, timeout_seconds) end - define_method(:attach) do |file, media_type, filename| - runtime.attach(file, media_type, filename) + define_method(:attach) do |file, media_type, filename, streamed_file| + runtime.attach(file, media_type, filename, streamed_file) end # Prints the list of modules that are included in the World diff --git a/lib/cucumber/runtime/user_interface.rb b/lib/cucumber/runtime/user_interface.rb index 433ef8273..1621d87f8 100644 --- a/lib/cucumber/runtime/user_interface.rb +++ b/lib/cucumber/runtime/user_interface.rb @@ -41,8 +41,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) - @visitor.attach(src, media_type, filename) + def attach(src, media_type, filename, streamed_file) + @visitor.attach(src, media_type, filename, streamed_file) end private diff --git a/spec/cucumber/glue/step_definition_spec.rb b/spec/cucumber/glue/step_definition_spec.rb index c377d7816..89514268c 100644 --- a/spec/cucumber/glue/step_definition_spec.rb +++ b/spec/cucumber/glue/step_definition_spec.rb @@ -63,7 +63,7 @@ def step_match(text) it 'calls a method on the world when specified with a symbol' do expect(registry.current_world).to receive(:with_symbol) - dsl.Given(/With symbol/, :with_symbol) + dsl.Given('With symbol', :with_symbol) run_step 'With symbol' end @@ -71,7 +71,7 @@ def step_match(text) it 'calls a method on a specified object' do allow(registry.current_world).to receive(:target) { target } - dsl.Given(/With symbol on block/, :with_symbol, on: -> { target }) + dsl.Given('With symbol on block', :with_symbol, on: -> { target }) expect(target).to receive(:with_symbol) @@ -81,7 +81,7 @@ def step_match(text) it 'calls a method on a specified world attribute' do allow(registry.current_world).to receive(:target) { target } - dsl.Given(/With symbol on symbol/, :with_symbol, on: :target) + dsl.Given('With symbol on symbol', :with_symbol, on: :target) expect(target).to receive(:with_symbol) @@ -102,17 +102,15 @@ def step_match(text) end it 'raises UndefinedDynamicStep when an undefined step is parsed dynamically' do - dsl.Given(/Outside/) do - steps %( - Given Inside - ) + dsl.Given('Outside') do + steps %(Given Inside) end expect { run_step 'Outside' }.to raise_error(Cucumber::UndefinedDynamicStep) end it 'raises UndefinedDynamicStep when an undefined step with doc string is parsed dynamically' do - dsl.Given(/Outside/) do + dsl.Given('Outside') do steps %( Given Inside """ @@ -125,7 +123,7 @@ def step_match(text) end it 'raises UndefinedDynamicStep when an undefined step with data table is parsed dynamically' do - dsl.Given(/Outside/) do + dsl.Given('Outside') do steps %( Given Inside | a | @@ -193,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) + expect(user_interface).to receive(:attach).with('wasup', 'text/x.cucumber.log+plain', nil, 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) + expect(user_interface).to receive(:attach).with('["Not", 1, "string"]', 'text/x.cucumber.log+plain', nil, nil) dsl.Given('Loud') { log ['Not', 1, 'string'] } run_step 'Loud'