Skip to content

Conversation

@jnewbery
Copy link
Contributor

@jnewbery jnewbery commented May 19, 2017

tl;dr:

  • temp directories now include the name of the test when run through the test_runner
  • passing a --tmpdirprefix to the test_runner allows test_runner to be called by cron/build jobs without risk of directory name collision

==================

By default, the functional tests create their temp directories in the $TMPDIR as follows:

$TMPDIR
   |
   -- test<rand_chars>
        |
        -- <port seed>
            |
            -- temp files
            -- ...

a --tmpdir argument can be passed into the individual test case, which causes the temp directory to be written to:

<specified --tmpdir>
   |
   -- <port seed>
        |
        -- temp files
        -- ...

If --tmpdir is passed to the test_runner, then it gets passed down to the indivdual tests and the directory structure is:

<specified --tmpdir>
   |
   -- <port seed 1>
        |
        -- temp files
        -- ...
   -- <port seed 2>
        |
        -- temp files
        -- ...
   -- ...

There are a few problems:

  • the directory names give no indication of which test was being run, so it's difficult to quickly find the temp directory for an indvidual test if using the test_runner
  • if a --tmpdir directory is passed to the test_runner and there are already directories in that tmpdir with names that clash with the port seed, then tests will fail when they try to use that subdirectory. This can be a problem when running test_runner as a cron job or build job with a specified tmpdir. Test failures in one build will cause subsequent runs to fail because the subdirectories are not cleaned up
  • even if a tmpdir is specified, test_framework will still create an empty temp directory $TMPDIR/test<rand_chars> (this is because tempfile.mkdtemp(prefix="test") is called even when --tmpdir is used).

This PR changes the temp dir structure so that:

  • if running the test directly:
    • if no --tmpdir is specified
      • temp directory is $TMPDIR/test<rand_chars> (as now, except without the <port seed> subdirectory)
    • if a --tmpdir is specified
      • temp directory is the specified tmpdir (as now, except without the <port seed> subdirectory)
  • if running through the test_runner
    • if no --tmpdirprefix is specified
      • temp directory is $TMPDIR/bitcoin_test_runner_<date>_<time>/<test_name>_portseed.
    • if a --tmpdirprefix is specified
      • temp directory is <tmpdirprefix>/bitcoin_test_runner_<date>_<time>/<test_name>_portseed.

<tmpdirprefix> will be cleaned up by the test_runner if it is empty at the end of all tests (ie if all tests have cleaned up their own temp directories).

@sdaftuar
Copy link
Member

ACK, this looks great!

@fanquake fanquake added the Tests label May 20, 2017
@laanwj
Copy link
Member

laanwj commented May 20, 2017

Concept ACK

@kallewoof
Copy link
Contributor

utACK b040243

1 similar comment
@maflcko
Copy link
Member

maflcko commented May 22, 2017

utACK b040243

@maflcko maflcko merged commit b040243 into bitcoin:master May 22, 2017
maflcko pushed a commit that referenced this pull request May 22, 2017
b040243 [tests] improve tmpdir structure (John Newbery)

Tree-SHA512: b21ad555c3c23a43087b893ad18bd2398e1df91b82c0bf1804d07fdb582600a1c339e6f4aaca58074e52f146f459943a0e492abc045b2666d4a3a7e0e455d6dd
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 24, 2019
b040243 [tests] improve tmpdir structure (John Newbery)

Tree-SHA512: b21ad555c3c23a43087b893ad18bd2398e1df91b82c0bf1804d07fdb582600a1c339e6f4aaca58074e52f146f459943a0e492abc045b2666d4a3a7e0e455d6dd
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants