Skip to content

Bugfix/retried envelope message#1844

Merged
luke-hill merged 12 commits intomainfrom
bugfix/retried_envelope_message
Apr 8, 2026
Merged

Bugfix/retried envelope message#1844
luke-hill merged 12 commits intomainfrom
bugfix/retried_envelope_message

Conversation

@luke-hill
Copy link
Copy Markdown
Contributor

Description

This fixes the situation where the retried envelope message is not reported correctly and will inadvertently report failed scenarios being ran "multiple" times.

This should also fix #1841

As part of this work. I also implemented the fix to introduce cucumber-query into the "message" formatter. Because of the way the codebase is structured, we can use cucumber-query independently in each formatter by just adding a single line.

Type of change

Please delete options that are not relevant.

  • Refactoring (improvements to code design or tooling that don't change behaviour)
  • Bug fix (non-breaking change which fixes an issue)

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.

  • Tests have been added for any changes to behaviour of the code
  • 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

… a single LOC and then have it repeatedly call this

This will have un-deduplicated calls where we need an interim message (Such as determining a message structure to find another), but this is a transitory interim phase and once we have removed all of the fake query structure and transitioned to full message protocol we can remove the un-deduplicated ones
@luke-hill
Copy link
Copy Markdown
Contributor Author

This needs a fix elsewhere first.

Copy link
Copy Markdown
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

Looks alright, but a few remarks that could be improved:

end

def output_envelope(envelope)
@repository.update(envelope)
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.

This looks redundant. The message formatter only ever has to write to file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is one of the "benefits" of the way the MessageBuilder is architected.

Every method calls output_envelope at the end of the notification which will use the newest defined one.

This allows me to avoid writing update in every item in the builder.

Later on I can probably make this better, but for now this means that everywhere inside cucumber-ruby we now have a fully updated query object - so now I can replace them (slowly later on).

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.

Then maybe a call is missing here.

will_be_retried: will_be_retried
)
)

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.

You don't have to create the TestCaseFinished twice. It should be possible to work out will_be_retried before creating the message based on the event and the previous messages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Might need to pair with you on this. For porting this in. Because I know it "should" be possible. But one of the issues I was finding here was trying to access the test_case_started.

The way in which "fake query" was architected by vincent / aurelien was to use events. But the cucumber-query uses messages. I could not find a reliable way to find a cucumber message using an id. i.e. if there was a cucumber-query method called
find_test_case_started_by_id or potentially I could call @repository.test_case_started[id]

WDYT. Is calling the repository directly viable? Let's maybe get some time together to pair on this when we're free if possible

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I only create the test case finished twice, because the first time gives me a TestCaseFinished message which I can then use to find the TestCaseStarted message (Using query)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed this up. The other items are needed to ensure that each message goes into the repository

@luke-hill luke-hill merged commit dfc6940 into main Apr 8, 2026
15 checks passed
@luke-hill luke-hill deleted the bugfix/retried_envelope_message branch April 8, 2026 17:06
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.

HTML Formatter is reporting scenarios as failed when they pass on a retry

2 participants