Failing to create tempdir in tests on windows

[Martin Packman]

A number of test suites fail on windows trying to create a tempdir that already exists:

<http://data.vapour.ws/juju-ci/products/version-2428/run-unit-tests-win2012-amd64/build-154/consoleText>

    PANIC: backups_test.go:1: downloadSuite.SetUpSuite

    ... Panic: Couldn't create temporary directory: mkdir C:\Users\ADMINI~1\AppData\Local\Temp/check-5765484004404056823: Cannot create a file when that file already exists. (PC=0x41593B)

    ...

    PANIC: state_test.go:205: StateFileSuite.TestStates

    test 0
    ... Panic: Couldn't create temporary directory: mkdir C:\Users\ADMINI~1\AppData\Local\Temp/check-5765484004404056823: Cannot create a file when that file already exists. (PC=0x41593B)

It's not surprising that we may fail to delete a tempdir on windows, but certainly the newly created ones should have unique names not the same long number suffix each time.

[Gabriel Samfira (gabriel-samfira)]

The following branch fixes a number of failing tests:

http://reviews.vapour.ws/r/1119

[Martin Packman (gz)]

So, I think the blame here is in this function from gopkg.in/check.v1/check.go l133:

func (td *tempDir) newPath() string {
        td.Lock()
        defer td.Unlock()
        if td.path == "" {
                var err error
                for i := 0; i != 100; i++ {
                        path := fmt.Sprintf("%s/check-%d", os.TempDir(), rand.Int())
                        if err = os.Mkdir(path, 0700); err == nil {
                                td.path = path
                                break
                        }
                }
                if td.path == "" {
                        panic("Couldn't create temporary directory: " + err.Error())
                }
        }
        result := filepath.Join(td.path, strconv.Itoa(td.counter))
        td.counter += 1
        return result
}

The interesting parts, rand.Int() gives the next item in a deterministic sequence, and this is not an independent Rand object or seeded in any way. As each test package is a separate process, all will be using the same sequence. As it tries 100 times to create a directory before the panic, this means once the first 100 directories have failed to be cleaned up, no new ones can be created.

[Bogdan Teleaga (bteleaga)]

The first thing that can be done to solve this is to clean the temporary directories after running the test suite.

Regarding the function in gocheck, while not ideal in the fact that it only manages to create a rather small finite number of directories, it however shows the real problem which is that more temporary directories do not get removed on windows than on other OS's, mostly due to the fact that most of the tests do not care to close whatever they opened before removing, which is going to not remove anything on windows. Actually fixing this is another issue and it would be pretty hard to do, just cleaning the temporary directories should do for now.

[Gabriel Samfira (gabriel-samfira)]

Cleaning the temporary directory might not be as easy. In the CI, the machine is always torn down after a run, do you get a clean %TMP% dir every time.

After a juju-core test run on windows I got 1667 directories left over. We probably will end up fixing this in the test suite. Until then, I would deffinitely use crypto/rand instead of math/rand. crypto/rand uses the operating system pseudorandom device, so we at least have a better chance of creating actual random integers.

Will look into the cleanup code tomorrow.