Skip to content

testhelper: Fix tests being order-dependent due to shared logging dir

Patrick Steinhardt requested to merge pks-test-log-dir-flakes into master

When the TEST_LOG_DIR environment variable is set then we write all logs into the directory pointed to by this variable. This variable is set in our CI so that we can retrieve more detailed logs in case any of the tests fails. The way this works though means that if the environment variable is set, then all tests will write logs to the same log directory. It is thus essentially shared state between all tests in case any of the tests happens to verify whether logs have been written.

And of course this happens: TestFetchInternalRemote_successful for example verifies that no log entry is written to "gitaly_hooks.log". This assertion frequently fails in our CI with an error related to Git's pre-receive hooks, even though this test doesn't run the pre-receive hook at all. This is impossible to debug locally though as we don't set up the shared logging directory by default, so as long as you don't happen to run with TEST_LOG_DIR set up you cannot ever reproduce the error.

Reduce the scope of this flake by sharding log directories by the test's name. So instead of writing into the same directory, each test will have its own log directory now. For one this fixes the flake, but on the other hand it also makes it so much easier to correlate log entries with failing tests as they are now clearly scoped by name.

This doesn't fully fix the issue though: if the same test name happens to exist across two different packages then we would still use the same log directory. Let's just accept that risk for now though -- it seems unlikely enough to be an issue in practice. And if it is we should maybe think about just ripping out the TEST_LOG_DIR handling altogether.

Merge request reports