-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Bugfix/retried envelope message #1844
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fc0a46f
4e0e3fe
4027d77
0469ad6
438468f
03bf56f
ab156bc
5183ed2
9735b0c
686c1e2
d937662
999245d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -15,6 +15,7 @@ def initialize(config) | |||
| end | ||||
|
|
||||
| def output_envelope(envelope) | ||||
| @repository.update(envelope) | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then maybe a call is missing here.
|
||||
| @io.write(envelope.to_json) | ||||
| @io.write("\n") | ||||
| end | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,6 +118,7 @@ def on_test_case_ready(event) | |
| ) | ||
| ) | ||
|
|
||
| # TODO: This may be a redundant update. But for now we're leaving this in whilst we're in the transitory phase | ||
| @repository.update(message) | ||
|
|
||
| # TODO: Switch this over to using the Repo Query object -> `test_step_by_id` | ||
|
|
@@ -126,9 +127,6 @@ def on_test_case_ready(event) | |
| @test_case_by_step_id[step.id] = event.test_case | ||
| end | ||
|
|
||
| # TODO: Once we're comfortable switching this over. Call @repository.update(message) alongside output_envelope | ||
| # however this may not be necessary as output_envelope may/should already be doing this? | ||
|
|
||
| output_envelope(message) | ||
| end | ||
|
|
||
|
|
@@ -186,7 +184,6 @@ def on_test_run_started(*) | |
| message = Cucumber::Messages::Envelope.new( | ||
| test_run_started: Cucumber::Messages::TestRunStarted.new( | ||
| timestamp: time_to_timestamp(Time.now), | ||
| # TODO: Switch this over to using the Query object -> `find_test_run_started` | ||
| id: @test_run_started_id | ||
| ) | ||
| ) | ||
|
|
@@ -211,10 +208,21 @@ def on_test_case_started(event) | |
|
|
||
| def on_test_step_created(event) | ||
| @pickle_id_step_by_test_step_id[event.test_step.id] = event.pickle_step.id | ||
| Cucumber::Messages::Envelope.new( | ||
| test_case: test_step_to_message(event.test_step) | ||
| ) | ||
| # TODO: This seems to output extra test cases that aren't expected | ||
| test_step_to_message(event.test_step) | ||
| # TODO: We need to determine what message to output here (Unsure - Placeholder added) | ||
| # message = Cucumber::Messages::Envelope.new( | ||
| # pickle: { | ||
| # id: '', | ||
| # uri: '', | ||
| # location: nil, | ||
| # name: '', | ||
| # language: '', | ||
| # steps: test_step_to_message(event.test_step), | ||
| # tags: [], | ||
| # ast_node_ids: [] | ||
| # } | ||
| # ) | ||
| # | ||
| # output_envelope(message) | ||
| end | ||
|
|
||
|
|
@@ -279,10 +287,18 @@ def create_exception_object(result, message_element) | |
| end | ||
|
|
||
| def on_test_case_finished(event) | ||
| test_case_started_id = test_case_started_id(event.test_case) | ||
| test_case_started_message = @repository.test_case_started_by_id[test_case_started_id] | ||
| max_attempts = @config.retry_attempts | ||
| # See "fake query" for reason this is index shifted | ||
| retries_attempted = test_case_started_message.attempt - 1 | ||
| will_be_retried = event.result.failed? && (retries_attempted < max_attempts) | ||
|
|
||
| message = Cucumber::Messages::Envelope.new( | ||
| test_case_finished: Cucumber::Messages::TestCaseFinished.new( | ||
| test_case_started_id: test_case_started_id(event.test_case), | ||
| timestamp: time_to_timestamp(Time.now) | ||
| test_case_started_id: test_case_started_id, | ||
| timestamp: time_to_timestamp(Time.now), | ||
| will_be_retried: will_be_retried | ||
| ) | ||
| ) | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't have to create the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 WDYT. Is calling the repository directly viable? Let's maybe get some time together to pair on this when we're free if possible
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.