Skip to content
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

Add exception to TestStepFinished TestRunFinished #122

Merged
merged 9 commits into from
Dec 16, 2022

Conversation

mpkorstanje
Copy link
Contributor

@mpkorstanje mpkorstanje commented Dec 11, 2022

🤔 What's changed?

To implement a message based JUnit XML Formatter (cucumber/junit-xml-formatter#3) we need to know the class and message of the exception that caused the scenario to fail. This information is often included implicitly in the message field but can not be reliably extracted.

For example if a scenario failed duo to a assertion exception the failure should be rendered as:

<failure message="[The scenario has pending or undefined step(s)](expected: <\"a\"> but was: <\"b\">)" type="org.opentest4j.AssertionFailedError">
  <![CDATA[

... Detailed stack trace here

]]>
</failure>

Currently however we can only render the stack trace part. The other information is not available.

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

Is this generic enough to work in other languages?

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

@davidjgoss
Copy link
Contributor

Is this generic enough to work in other languages?

I think it's okay at least from a JS persective. We can always provide a generic fallback if those fields are not present?

You mentioned the flatness of the structure before, I guess the alternative would be an exception sub-object with type and message fields - this would then be a natural place for additional granular detail about the exception to go e.g. actual vs expected when we eventually get to that.

@mpkorstanje
Copy link
Contributor Author

Good one. There are some proposals to add more information about exceptions.

@mpkorstanje
Copy link
Contributor Author

I've added an Exception object with type and message fields. I looked at adding stacktrace and expected and actual but it's a bit too much work to see that every language can do with stack traces.

@mpkorstanje mpkorstanje marked this pull request as ready for review December 16, 2022 17:05
@mpkorstanje mpkorstanje changed the title Add exception type and message Add exception type to test step finished and test run finished events Dec 16, 2022
@mpkorstanje mpkorstanje changed the title Add exception type to test step finished and test run finished events Add exception to TestStepFinished TestRunFinished Dec 16, 2022
@mpkorstanje mpkorstanje merged commit b61f824 into main Dec 16, 2022
@mpkorstanje mpkorstanje deleted the add-exception-type-and-message branch December 16, 2022 17:18
@davidjgoss
Copy link
Contributor

What's the use case for the exception on TestRunFinished out of interest?

@mpkorstanje
Copy link
Contributor Author

To inform about exceptions that didn't happen in steps or global hooks. For example in Cucumber JVM this would be related to creating the dependency injection container.

Cucumbers execution model can be seen as a tree of containers. Each container has a started and finished event. Each finished event should have an exception for exceptions that occurred in that container.

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.

None yet

2 participants