citrusframework / citrus

Framework for automated integration tests with focus on messaging integration
https://citrusframework.org
Apache License 2.0
459 stars 136 forks source link

CitrusSpringExtension for JUnit5 does not always log the generatedReports #816

Closed tschlat closed 1 year ago

tschlat commented 2 years ago

Citrus Version 3.1.0

Expected behavior Citrus logs the test execution statistics via TestReporters.generateReports at the end of a test suite execution.

Actual behavior If Logback is for example used as an implementation for logging, the statistics are not deterministically logged. The reason for this is that this specific logging is triggered by a VM ShutDownHook which is undeterministically executed before or after the SpringApplicationShutdownHook. The later is responsible for unregistration of Logback LogAppenders. If the appenders are unregistered before the statistics logging, the log.info() call will run into no appenders and thus logging is empty.

One solution would be to add a @PreDestroy Annotation to TestReporters.generateReports to trigger the reporting. Alternatively to not mess up TestReporters with Spring specific annotations, a Bean with a @PreDestroy method could be registered dynamically within the CitrusSpringExtension which takes care of finishing.

Advice would be appreciated before I provide a fix via PR.

christophd commented 2 years ago

@tschlat many thanks for the detailed analysis. I think the solution with @PreDestroy annotation is a valid fix. As the issue is related to Spring based projects I would make sure to only use this solution when the TestReporters are loaded via Spring. In non Spring projects we can keep the ShutDownHook based reporting I guess

tschlat commented 1 year ago

We were just investigating on this issue to provide a fix but it seems like this has already beenfixed in 4.0.0. Prior to the fix, the mentioned ShutdownHook was added in CitrusExtension.beforeAll().

This is the code of V 3.1 if (afterSuite) { afterSuite = false; Runtime.getRuntime().addShutdownHook(new Thread(() -> CitrusExtensionHelper.getCitrus(extensionContext).afterSuite(SUITE_NAME))); }

The current implementation calls the Citrus.afterSuite() directly in CitrusExtension.afterAll() which seems to be the right solution:

extensionContext.getTestClass().map(Class::getName).or(() -> extensionContext.getTestMethod().map(meth -> meth.getDeclaringClass().getName()+":"+meth.getName()) ).ifPresentOrElse(suiteName -> CitrusExtensionHelper .getCitrus(extensionContext) .afterSuite(suiteName, tags), () -> CitrusExtensionHelper .getCitrus(extensionContext) .afterSuite(SUITE_NAME, tags) );

christophd commented 1 year ago

It is funny, because yesterday I was fixing some more issue with JUnit5 and Citrus reports. It turned out that the afterAll() callback is also not the right place to trigger the Citrus report because this does not ensure that the report has all tests and it is called after the last test in the suite has finished.

So I ended up using a documented workaround with closable resource to call the reporting after the suite has finished. Also I made sure to not loos test results when e.g. the Spring application context is dirty and gets reloaded.

christophd commented 1 year ago

Here is the commit as a reference https://github.com/citrusframework/citrus/commit/1252851a6cf08bd0d1024900fe0947da09dce5ad

christophd commented 1 year ago

I think we can call this one fixed