Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move or remove will_be_retried from TestStepResult #902

Closed
aslakhellesoy opened this issue Feb 15, 2020 · 8 comments · Fixed by #1631
Closed

Move or remove will_be_retried from TestStepResult #902

aslakhellesoy opened this issue Feb 15, 2020 · 8 comments · Fixed by #1631
Labels
library: cucumber-messages 🧷 pinned Tells Stalebot not to close this issue

Comments

@aslakhellesoy
Copy link
Contributor

See 4baf62d#r37297905 for context.

Results are only attached to steps, which is why the message was renamed.

I’m not sure where the will_be_retried property is written or read, I think somewhere in Cucumber.js @charlierudolph @davidjgoss?

Would it make more sense to place it on TestCaseFinished?

@davidjgoss
Copy link
Contributor

davidjgoss commented Feb 16, 2020

Agree that will_be_retried makes sense on TestCaseFinished given it represents a finished attempt. I defer to Charlie though.

Cucumber.js currently uses TestResult as both a step result and test case attempt result, in pickle_runner.ts - in the version of messages we are currently on, TestCaseFinished still has a result, on which we set the overall duration and status for the attempt. From the commit where it was removed, is the idea now to use cucumber-query to resolve that kind of aggregated data rather than emit it on the messages?

@charlierudolph
Copy link
Member

charlierudolph commented Feb 16, 2020

I'm fine moving it from part of TestResult to TestCaseFinished. And yes definitely interested how we should be getting a test case attempt's results when we don't have a result field on there anymore

@aslakhellesoy
Copy link
Contributor Author

@davidjgoss yes, I think @cucumber/query should be used for this. It currently doesn't track TestCaseStarted#id / TestStepFinished#testCaseStartedId, but it would be trivial to make it do so.

@charlierudolph I'm thinking we could add a testCaseStartedId parameter to #getPickleStepTestStepResults and #getPickleTestStepResults. Then the returned array can be passed through #getWorstTestStepResult to get a single result.

WDYT?

@charlierudolph
Copy link
Member

Sounds good to me.

@stale
Copy link

stale bot commented Apr 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the ⌛ stale Will soon be closed by stalebot unless there is activity label Apr 17, 2020
@stale
Copy link

stale bot commented Apr 25, 2020

This issue has been automatically closed because of inactivity. You can support the Cucumber core team on opencollective.

@stale stale bot closed this as completed Apr 25, 2020
@davidjgoss davidjgoss reopened this May 1, 2020
@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label May 1, 2020
@stale
Copy link

stale bot commented Jul 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the ⌛ stale Will soon be closed by stalebot unless there is activity label Jul 3, 2020
@stale
Copy link

stale bot commented Jul 11, 2020

This issue has been automatically closed because of inactivity. You can support the Cucumber core team on opencollective.

@stale stale bot closed this as completed Jul 11, 2020
@davidjgoss davidjgoss reopened this Jul 12, 2020
@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Jul 12, 2020
@davidjgoss davidjgoss added the 🧷 pinned Tells Stalebot not to close this issue label Jul 12, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
library: cucumber-messages 🧷 pinned Tells Stalebot not to close this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants