Play nicely with post-test scripts and always printing server logs

ENVIRONMENT.md

@@ -4,7 +4,7 @@
 Complement is configured exclusively through the use of environment variables. These variables are described below.
 
 #### `COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS`
-If 1, always prints the Homeserver container logs even on success.  
+If 1, always prints the Homeserver container logs even on success. When used with COMPLEMENT_ENABLE_DIRTY_RUNS, server logs are only printed once for reused deployments, at the very end of the test suite.  
 - Type: `bool`
 - Default: 0
 
@@ -22,7 +22,7 @@ If 1, prints out more verbose logging such as HTTP request/response bodies.
 - Default: 0
 
 #### `COMPLEMENT_ENABLE_DIRTY_RUNS`
-If 1, eligible tests will be provided with reusable deployments rather than a clean deployment. Eligible tests are tests run with `Deploy(t, numHomeservers)`. If enabled, COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS and COMPLEMENT_POST_TEST_SCRIPT are ignored.  Enabling dirty runs can greatly speed up tests, at the cost of clear server logs and the chance of tests polluting each other. Tests using `OldDeploy` and blueprints will still have a fresh image for each test. Fresh images can still be desirable e.g user directory tests need a clean homeserver else search results can be polluted, tests which can blacklist a server over federation also need isolated deployments to stop failures impacting other tests. For these reasons, there will always be a way for a test to override this setting and get a dedicated deployment.  Eventually, dirty runs will become the default running mode of Complement, with an environment variable to disable this behaviour being added later, once this has stablised.  
+If 1, eligible tests will be provided with reusable deployments rather than a clean deployment. Eligible tests are tests run with `Deploy(t, numHomeservers)`. If enabled, COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS and COMPLEMENT_POST_TEST_SCRIPT are run exactly once, at the end of all tests in the package. The post test script is run with the test name "COMPLEMENT_ENABLE_DIRTY_RUNS", and failed=false.  Enabling dirty runs can greatly speed up tests, at the cost of clear server logs and the chance of tests polluting each other. Tests using `OldDeploy` and blueprints will still have a fresh image for each test. Fresh images can still be desirable e.g user directory tests need a clean homeserver else search results can be polluted, tests which can blacklist a server over federation also need isolated deployments to stop failures impacting other tests. For these reasons, there will always be a way for a test to override this setting and get a dedicated deployment.  Eventually, dirty runs will become the default running mode of Complement, with an environment variable to disable this behaviour being added later, once this has stablised.  
 - Type: `bool`
 - Default: 0
 
@@ -40,7 +40,7 @@ A list of space separated blueprint names to not clean up after running. For exa
 - Type: `[]string`
 
 #### `COMPLEMENT_POST_TEST_SCRIPT`
-An arbitrary script to execute after a test was executed and before the container is removed. This can be used to extract, for example, server logs or database files. The script is passed the parameters: ContainerID, TestName, TestFailed (true/false)  
+An arbitrary script to execute after a test was executed and before the container is removed. This can be used to extract, for example, server logs or database files. The script is passed the parameters: ContainerID, TestName, TestFailed (true/false). When combined with COMPLEMENT_ENABLE_DIRTY_RUNS, the script is called exactly once at the end of the test suite, and is called with the TestName of "COMPLEMENT_ENABLE_DIRTY_RUNS" and TestFailed=false.  
 - Type: `string`
 - Default: ""
 

internal/config/config.go

@@ -36,7 +36,9 @@ type Complement struct {
 	DebugLoggingEnabled bool
 	// Name: COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS
 	// Default: 0
-	// Description: If 1, always prints the Homeserver container logs even on success.
+	// Description: If 1, always prints the Homeserver container logs even on success. When used with
+	// COMPLEMENT_ENABLE_DIRTY_RUNS, server logs are only printed once for reused deployments, at the very
+	// end of the test suite.
 	AlwaysPrintServerLogs bool
 	// Name: COMPLEMENT_SHARE_ENV_PREFIX
 	// Description: If set, all environment variables on the host with this prefix will be shared with
@@ -90,7 +92,8 @@ type Complement struct {
 	// Default: 0
 	// Description: If 1, eligible tests will be provided with reusable deployments rather than a clean deployment.
 	// Eligible tests are tests run with `Deploy(t, numHomeservers)`. If enabled, COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS
-	// and COMPLEMENT_POST_TEST_SCRIPT are ignored.
+	// and COMPLEMENT_POST_TEST_SCRIPT are run exactly once, at the end of all tests in the package. The post test script
+	// is run with the test name "COMPLEMENT_ENABLE_DIRTY_RUNS", and failed=false.
 	//
 	// Enabling dirty runs can greatly speed up tests, at the cost of clear server logs and the chance of tests
 	// polluting each other. Tests using `OldDeploy` and blueprints will still have a fresh image for each test.
@@ -109,7 +112,9 @@ type Complement struct {
 	// Default: ""
 	// Description: An arbitrary script to execute after a test was executed and before the container is removed.
 	// This can be used to extract, for example, server logs or database files. The script is passed the parameters:
-	// ContainerID, TestName, TestFailed (true/false)
+	// ContainerID, TestName, TestFailed (true/false). When combined with COMPLEMENT_ENABLE_DIRTY_RUNS, the script is
+	// called exactly once at the end of the test suite, and is called with the TestName of "COMPLEMENT_ENABLE_DIRTY_RUNS"
+	// and TestFailed=false.
 	PostTestScript string
 }
 

internal/docker/deployment.go

@@ -54,6 +54,16 @@ func (hsDep *HomeserverDeployment) SetEndpoints(baseURL string, fedBaseURL strin
 	}
 }
 
+// DestroyAtCleanup destroys the entire deployment. It should be called at cleanup time for dirty
+// deployments only. Handles configuration options for things which should run at container destroy
+// time, like post-run scripts and printing logs.
+func (d *Deployment) DestroyAtCleanup() {
+	if !d.Dirty {
+		return
+	}
+	d.Deployer.Destroy(d, d.Deployer.config.AlwaysPrintServerLogs, "COMPLEMENT_ENABLE_DIRTY_RUNS", false)
+}
+
 // Destroy the entire deployment. Destroys all running containers. If `printServerLogs` is true,
 // will print container logs before killing the container.
 func (d *Deployment) Destroy(t *testing.T) {

test_package.go

@@ -84,6 +84,12 @@ func NewTestPackage(pkgNamespace string) (*TestPackage, error) {
 }
 
 func (tp *TestPackage) Cleanup() {
+	// any dirty deployments need logs printed and post scripts run
+	tp.existingDeploymentsMu.Lock()
+	for _, dep := range tp.existingDeployments {
+		dep.DestroyAtCleanup()
+	}
+	tp.existingDeploymentsMu.Unlock()
 	tp.complementBuilder.Cleanup()
 }