Conversation
… 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
|
This needs a fix elsewhere first. |
mpkorstanje
left a comment
There was a problem hiding this comment.
Looks alright, but a few remarks that could be improved:
| end | ||
|
|
||
| def output_envelope(envelope) | ||
| @repository.update(envelope) |
There was a problem hiding this comment.
This looks redundant. The message formatter only ever has to write to file.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Then maybe a call is missing here.
| will_be_retried: will_be_retried | ||
| ) | ||
| ) | ||
|
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Fixed this up. The other items are needed to ensure that each message goes into the repository
…ID to fetch the initial message to then determine retry status
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.
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.
bundle exec rubocopreports no offenses