Skip to content

Commit

Permalink
testing: revert GOTMPDIR hack for setting testscript workdir root (#673)
Browse files Browse the repository at this point in the history
In 854efae we included a temporary hack for setting the destination of
artefacts for testscript tests, via GOTMPDIR. In this commit we undo
that hack.

Motivation for the change best described in:

rogpeppe/go-internal@bc89b17
  • Loading branch information
myitcv committed Jan 15, 2020
1 parent 29461d3 commit 309f57b
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 29 deletions.
17 changes: 4 additions & 13 deletions cmd/govim/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,10 @@ func TestMain(m *testing.M) {
}

func TestScripts(t *testing.T) {
// TODO our approach with setting the workdir root via os.Setenv("GOTMPDIR")
// is hacky and gross. Working out a cleaner approach with rogpeppe, likely
// passing in such a value via Params
t.Parallel()
var workdir string
if envworkdir := os.Getenv(testsetup.EnvTestscriptWorkdirRoot); envworkdir == "" {
// i.e. we are not going to call os.Setenv below
t.Parallel()
} else {
if envworkdir := os.Getenv(testsetup.EnvTestscriptWorkdirRoot); envworkdir != "" {
workdir = filepath.Join(envworkdir, "cmd", "govim"+raceOrNot())
defer os.Setenv("GOTMPDIR", os.Getenv("GOTMPDIR"))
}

var waitLock sync.Mutex
Expand Down Expand Up @@ -92,14 +86,11 @@ func TestScripts(t *testing.T) {
if workdir != "" {
workdir = filepath.Join(workdir, entry.Name())
os.MkdirAll(workdir, 0777)
os.Setenv("GOTMPDIR", workdir)
}
t.Run(entry.Name(), func(t *testing.T) {
// Do not call t.Parallel here. We currently rely on this being
// run on the test goroutine in order that os.Setenv is not racey
params := testscript.Params{
TestWork: workdir != "",
Dir: filepath.Join("testdata", entry.Name()),
WorkdirRoot: workdir,
Dir: filepath.Join("testdata", entry.Name()),
Cmds: map[string]func(ts *testscript.TestScript, neg bool, args []string){
"sleep": testdriver.Sleep,
"errlogmatch": testdriver.ErrLogMatch,
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
github.com/fsnotify/fsnotify v1.4.7
github.com/kr/pretty v0.1.0
github.com/myitcv/vbash v0.0.4
github.com/rogpeppe/go-internal v1.5.1
github.com/rogpeppe/go-internal v1.5.2
golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee
golang.org/x/sync v0.0.0-20190423024810-112230192c58
golang.org/x/sys v0.0.0-20190429190828-d89cdac9e872 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ github.com/rogpeppe/clock v0.0.0-20190514195947-2896927a307a h1:3QH7VyOaaiUHNrA9
github.com/rogpeppe/clock v0.0.0-20190514195947-2896927a307a/go.mod h1:4r5QyqhjIWCcK8DO4KMclc5Iknq5qVBAlbYYzAbUScQ=
github.com/rogpeppe/go-internal v1.2.1/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.5.1 h1:asQ0uD7BN9RU5Im41SEEZTwCi/zAXdMOLS3npYaos2g=
github.com/rogpeppe/go-internal v1.5.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
github.com/rogpeppe/go-internal v1.5.2 h1:qLvObTrvO/XRCqmkKxUlOBc48bI3efyDuAZe25QiF0w=
github.com/rogpeppe/go-internal v1.5.2/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
Expand Down
16 changes: 3 additions & 13 deletions govim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,16 @@ func TestMain(m *testing.M) {
}

func TestScripts(t *testing.T) {
// TODO our approach with setting the workdir root via os.Setenv("GOTMPDIR")
// is hacky and gross. Working out a cleaner approach with rogpeppe, likely
// passing in such a value via Params
t.Parallel()
workdir := os.Getenv(testsetup.EnvTestscriptWorkdirRoot)
if workdir == "" {
// i.e. we are not going to call os.Setenv below
t.Parallel()
} else {
os.MkdirAll(workdir, 0777)
os.Setenv("GOTMPDIR", workdir)
defer os.Setenv("GOTMPDIR", os.Getenv("GOTMPDIR"))
}

var waitLock sync.Mutex
var waitList []func() error

t.Run("scripts", func(t *testing.T) {
testscript.Run(t, testscript.Params{
TestWork: workdir != "",
Dir: "testdata",
WorkdirRoot: workdir,
Dir: "testdata",
Cmds: map[string]func(ts *testscript.TestScript, neg bool, args []string){
"sleep": testdriver.Sleep,
"errlogmatch": testdriver.ErrLogMatch,
Expand Down

0 comments on commit 309f57b

Please sign in to comment.