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

JUnit run notifier will fire test finished as well after test failure… #909

Merged
merged 2 commits into from Apr 25, 2016

Conversation

Projects
None yet
2 participants
@sgo

sgo commented Apr 23, 2016

I created a patch for issue #908 to help out.

In summary the FitNesseRunner doesn't work properly with the Gradle build tool. Reason for this is they expect an invocation of fireTestFinished for every fireTestStarted even if you already invoked fireTestFailure.
They do this as suggested by the Javadoc of RunNotifier.fireTestFinished (http://junit.sourceforge.net/javadoc/org/junit/runner/notification/RunNotifier.html#fireTestFinished%28org.junit.runner.Description%29)

The effect is that any failures or errors observed in a fitnesse run are ignored by Gradle's test result listener.

I added tests to JUnitRunNotifierResultsListenerTest to cover this behavior.

Finally, I'd like to bring your attention to the fact I modified the 2 existing tests slightly to illustrate the summaries passed into testComplete don't matter. They only matter when the test result succeeds and the summary still insists there were failures or errors of some kind.
I don't know if this use case makes sense so I made sure to cover both approaches when adding my own tests.

sgo
JUnit run notifier will fire test finished as well after test failure…
…s as some result listeners expect this behavior.
@amolenaar

This comment has been minimized.

Show comment
Hide comment
@amolenaar

amolenaar Apr 24, 2016

Collaborator

Thanks for the PR. I have one small remark, apart from that it looks nice.

Collaborator

amolenaar commented Apr 24, 2016

Thanks for the PR. I have one small remark, apart from that it looks nice.

sgo
refactored JUnitRunNotifierResultsListener.testComplete to fire test …
…finished once at the end instead of for every branch of the if-else structure.
@sgo

This comment has been minimized.

Show comment
Hide comment
@sgo

sgo Apr 25, 2016

I updated the PR. If there's anything else I'll get to it when I find some time.

sgo commented Apr 25, 2016

I updated the PR. If there's anything else I'll get to it when I find some time.

@amolenaar amolenaar added this to the Next release milestone Apr 25, 2016

@amolenaar

This comment has been minimized.

Show comment
Hide comment
@amolenaar

amolenaar Apr 25, 2016

Collaborator

Awesome!

Collaborator

amolenaar commented Apr 25, 2016

Awesome!

@amolenaar amolenaar merged commit aeb2a2e into unclebob:master Apr 25, 2016

amolenaar added a commit that referenced this pull request Apr 25, 2016

Merge pull request #909 from sgo/issue-908
JUnit run notifier will fire test finished as well after test failure…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment