always copy logs to circle in finally

[pnepywoda]

call after() (exactly 1 time)

[changelog-app]

Generate changelog in changelog/@unreleased

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

always copy logs to circle in finally

doesnt work - jupiter always calls after anyway

Check the box to generate changelog(s)

• Generate changelog entry

[pnepywoda]

Running DockerComposeExtensionIntegrationTestWithFailingCommand results in:

[main] ERROR com.palantir.docker.compose.reporting.RunRecorder - EnhancedDockerComposeRule has failed in after()
com.palantir.logsafe.exceptions.SafeIllegalArgumentException: Some required fields have not been set: {missingFields=[startTime]}
	at com.palantir.docker.compose.report.DockerComposeRun.validateFields(DockerComposeRun.java:174)
	at com.palantir.docker.compose.report.DockerComposeRun.<init>(DockerComposeRun.java:49)
	at com.palantir.docker.compose.report.DockerComposeRun.<init>(DockerComposeRun.java:24)
	at com.palantir.docker.compose.report.DockerComposeRun$Builder.build(DockerComposeRun.java:306)
	at com.palantir.docker.compose.reporting.RunRecorder.after(RunRecorder.java:73)
	at com.palantir.docker.compose.DockerComposeManager.after(DockerComposeManager.java:269)
	at com.palantir.docker.compose.DockerComposeExtension.afterAll(DockerComposeExtension.java:45)

because we reset the runRecorder in after

[CRogers]

We should make sure calling after() multiple times is ok. I think the easiest way to do this to either record a boolean flag for whether after has been called before (and then not run it again) or use a memoised supplier to just run the code in after once.

[CRogers]

I would avoid having behaviour like this in junit5 version only. Instead this should live in DockerComposeManager.before() so both junit4 and 5 have the same behaviour

[CRogers]

Actually, I've just seen the junit4 code has something similar but needs to be slightly different as it uses the Statement style - this could stay. A little worried about the rethrowing the exception meaning this piece of code is missing in the stacktrace (perhaps addSupressed could help here by appending on an extra exception, if we need to keep the existing exception for backcompat reasons).

[pnepywoda]

@CRogers I can do the call-after-once fix you suggested but do you know why the current integration test isn't failing here? I guess it's not running my new DockerComposeExtensionIntegrationTestWithFailingCommand ?

[CRogers]

I think this should live in DockerComposeManager, not DockerComposeExtension that way DockerComposeRule gets this behaviour too.