Unit tests do not run in their own sandbox

[Emmanuel Blot]

Unit tests use the system temporary directory to create sub dirs and files. This leads to two distinct issues:

1. Two test sessions running at once on the same host may collide, as they create, update and remove files and directories within the same top directory
2. If the temporary directory already contains files or dirs from a previous test session, the current test session fails within the setUp() methods. This issue causes the Python unittest framework not to call the clean up tearDown() methods, leaving the faulty directory unfixed. Any subsequent unit test session keeps failing until manual removal of the directory is performed.

A unique temporary directory could be created and used for every single test session, to avoid those issues.

[Remy Blank]

This has annoyed me quite often, too. Great to see you pick it up!

[Christian Boos]

We already have environment variables like TRAC_TEST_DB_URI, so I'd suggest using the TRAC_TEST_TMP_DIR spelling over TRACTESTTMPDIR (also for the global variable, for consistency). More importantly, we shouldn't rmtree that folder without checking that it looks like the kind of temp dir we need, e.g. if it is either an empty dir or contains only 'trac-*' entries (and then we should stick to this convention of populating it with entries starting with trac- as I believe we do now). I think it's too dangerous otherwise.

Likewise, FUNCTESTTMPDIR could be renamed TRAC_TEST_FUNC_TMP_DIR. As that one can't be set via an environment variable, we don't need to do the same kind of paranoid check as above (other than against the empty string which means the current dir) before rmtreeing it.

I see you also kicked the "testenv%s" % port logic, which was probably there for a reason… (CC'ing Eli to know more about this, whether it's still useful or not).

[Eli Carter]

Part of the logic of having the test environment in the source tree was so that you could examine it after the tests completed, especially if the tests failed. If I remember correctly, if a test fails we save a copy of the page we're examining when it fails… and I believe that's going to wind up somewhere under the testenv tree.

The "testenv%s" % port logic was to give you the ability to use the tests to create separate example trac environments so you could compare and contrast them or run them in parallel. (They do need to listen on different ports.) But in the default case where you don't specify a port, I didn't want to wind up leaving 1000 different test directories lying around, I just used "testenv" so it would clean up the previous test's results.

[Christian Boos]

Replying to ecarter:

… if a test fails we save a copy of the page we're examining when it fails… and I believe that's going to wind up somewhere under the testenv tree.

Ah right, into <srcdir>/testenv/log (see twill_write_html()​), and indeed looking at those files is often really necessary to understand why a test failed, so we need to continue to support that in some way.

The fact that the testenv ends up elsewhere won't be a problem, as the full filename will be reported in the error message, but obviously removing those files after a test run is not going to help. One possible solution would be create those files into another location (<srcdir>/log or <srcdir>/build/log, with a process id stamp to allow for concurrent runs? Same thing would be needed for testenv.log and functional-testing.log, btw).

Note that sometimes even having the full test environment can be useful for figuring out what the problem was… maybe we could avoid removing it if the TRAC_TEST_FUNC_TMP_DIR environment variable is set? Therefore setting TRAC_TEST_FUNC_TMP_DIR=testenv would emulate the current behavior.

The "testenv%s" % port logic was to give you the ability to use the tests to create separate example trac environments so you could compare and contrast them or run them in parallel. (They do need to listen on different ports.) But in the default case where you don't specify a port, I didn't want to wind up leaving 1000 different test directories lying around, I just used "testenv" so it would clean up the previous test's results.

Good. However I don't see yet how to specify that port (when launching the functional test suite I mean), is that part missing?

[osimons]

It is not just the logs, it is the complete Trac + Subversion test environment that can also be just launched separately during and after test runs and contains the functional results of whatever tests are run. Data, attachments, configuration settings and more. It is very useful.

I also subclass and reuse the functional testing infrastructure, most notably for the RPC plugin. I do specify a separate location and custom port though, so as to not interfere with Trac test environment. I expect that flexibility to continue to work.

​https://www.coderesort.com/u/simon/blog/2010/09/23/tracrpc_development

[Christian Boos]

Yes, fully agree for functional tests. IIUC, leaving things behind and clearing it on the next run is the current behavior, which is only problematic if we'd like to get concurrent runs from the same checkout. So do we really want to change this?

Then I'm wondering if this also applies to unit-tests. I don't remember having ever felt the need to look into a test environment created by a unit-test, after a failure or an error.

[osimons]

No, it doesn't apply for unit tests in my opinion either. I'm fine with patching temporary directory handling for that - particularly as the various mocks and stubs don't really leave much of value in the file system anyway.

[Eli Carter]

Replying to cboos:

Good. However I don't see yet how to specify that port (when launching the functional test suite I mean), is that part missing?

It existed at some point, though I'm not finding it either.

[Remy Blank]

Preparing for 1.0.

[Christian Boos]

Moving enhancements to dev.