Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Simplify temp dir creation in tests #882

Merged
merged 2 commits into from
Jul 26, 2017
Merged

Conversation

tkrajina
Copy link
Contributor

If the tempdir is evaluated on creation, there is no need for special
handling of "darwin" (OSX) in NewTestProject()

What does this do / why do we need it?

It simplifies the code in NewTestProject() (internal/test/integration_testproj.go) by removing special handling when runtime.GOOS == "darwin".

The problem is that tempfiles[1] on OSX are symlinks and it's just simpler to EvalSymlinks when the test directory is created than having special handling for "darwin" later.

[1] An example temp directory on OSX:

tmpDir: /var/folders/np/f6xzpfd94217nvgdpmwt_qbr0000gn/T/gotest259673386
...it symlinks to: /private/var/folders/np/f6xzpfd94217nvgdpmwt_qbr0000gn/T/gotest259673386

If the tempdir is evaluated on creation, there is no need for special
handling of "darwin" (OSX) in NewTestProject()
@tkrajina
Copy link
Contributor Author

tkrajina commented Jul 22, 2017

Note, this PR don't solve any bug in the dep tool. It just makes the testing code simpler, and it makes the OSX build on travis simpler to handle. At the moment both the GOPATH and the project paths on OSX are symlinks. Any change trying to eval only one of those symlinks (either the GOPATH or the project path) will cause to fourth (OSX) build to fail.

See, for example the build for #834: https://travis-ci.org/golang/dep/builds/254211644?utm_source=github_status&utm_medium=notification

Same to the first build in #812


// Fox for OSX where the tempdir is a symlink:
p.tempdir, err = filepath.EvalSymlinks(p.tempdir)
p.Must(err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not wrap this in if runtime.GOOS == "darwin" { ... }?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...for a hypothetical new OS/platform which also creates temp files as symlinks? :)

PS. Added now.

Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep yep, this seems reasonable

@sdboyer sdboyer merged commit 7d36525 into golang:master Jul 26, 2017
@sdboyer
Copy link
Member

sdboyer commented Jul 26, 2017

thanks! 🎉

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.

4 participants