Skip to content

The Formatter::HTML should not inherit from the MessageBuilder#1899

Open
brasmusson wants to merge 2 commits into
mainfrom
bugfix/html-formatter-no-inheritance
Open

The Formatter::HTML should not inherit from the MessageBuilder#1899
brasmusson wants to merge 2 commits into
mainfrom
bugfix/html-formatter-no-inheritance

Conversation

@brasmusson

Copy link
Copy Markdown
Contributor

Description

  • The Runtime creates the MessageBuilder to always output the envelope it creates from event to the event bus. Therefore a formatter only need to listen to the :envelope events on the event bus.

This change should have been part of #1865. Since it was missed there #1890 occurred. #1891 fixed the symptom of #1890 but failed to remove the inheritance fo the Formatter::HTML

Type of change

Please delete options that are not relevant.

  • Refactoring (improvements to code design or tooling that don't change behaviour)

Please add an entry to the relevant section of CHANGELOG.md as part of this pull request.

Checklist:

Your PR is ready for review once the following checklist is
complete. You can also add some checks if you want to.

  • New and existing tests are passing locally and on CI
  • bundle exec rubocop reports no offenses
  • RDoc comments have been updated
  • CHANGELOG.md has been updated

* The Runtime creates the MessageBuilder to always output the envelope
  it creates from event to the event bus. Therefore a formatter only
  need to listen to the :envelope events on the event bus.

@luke-hill luke-hill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to ensure we're calling repository update if we no longer call super. Especially with the missing MessageHandler method. Which ensures that attachments are good

@brasmusson

Copy link
Copy Markdown
Contributor Author

The HTML formatter does not have a repository, it just listens to the message stream and forwards the messages to the HTML-formatter from the html-formatter repo. It is the MessageBuilder instance in the Runtime (https://github.com/cucumber/cucumber-ruby/pull/1899/changes#diff-fc62aa05d38336fb22819196433b9acb4370acadd10dbc201a48ccbd5980df6dR178) that ensures that the full message stream is available on the event-bus and that the attachments are good.

@luke-hill

Copy link
Copy Markdown
Contributor

So will the on_envelope handler for the MessageBuilder still be called when using the HTML Formatter?

Reason I ask is because we need the store_... method to be called so that attachments have their source preserved.

@brasmusson

Copy link
Copy Markdown
Contributor Author

The Runtime creates an instance of MessageBuilder, it will register its on_envelope method on the EventBus for envelope events. When a envelope event is passed to the EventBus the on_envelope method of the MessageBuilder instance will be called. This will happen regardless of which formatters that are used during the test run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants