search

eclipse / n4js

Eclipse Public License 1.0
30 stars 27 forks source link

Replace fireTestRunStarted/Finished by fireTestSuiteStarted/Finished in XtFileRunner #2581

Closed jean-andre-gauthier closed 9 months ago

jean-andre-gauthier commented 9 months ago

This PR replaces calls to fireTestRunStarted/Finished by fireTestSuiteStarted/Finished in XtFileRunner. According to the JUnit 4 docs, fireTestRunStarted/Finished are not supposed to be invoked by custom runners, which should use fireTestSuiteStarted/Finished instead.

jean-andre-gauthier commented 9 months ago

@marcphilipp could you please double-check if the changes correspond to what you had in mind?

mmews-n4 commented 9 months ago

@jean-andre-gauthier @marcphilipp Your initiative and help is appreciated. Thanks! Regarding the test output: I also was wondering why the test suite was started twice. When you un-draft this PR, I will have a look at it right away.

jean-andre-gauthier commented 9 months ago

Hi @mmews-n4 @marcphilipp, thanks for your comments.

The wrapping testSuiteStarted/Finished in the tests is triggered by ParentRunner.run for XtTestSetupTestMockup. I've just double-checked, the behaviour hasn't changed there, these callbacks were already being called before my changes. They didn't show up in the tests though, because AbstractXtParentRunnerTest.TestRunSimulator wasn't tracking them up until now. To make things more explicit, I have added the description to the event key in TestRunSimulator.testSuiteStarted/Finished.

Please let me know if that looks fine to you?

mmews-n4 commented 9 months ago

Thanks again!