From f55fd4aefc39b24d6fbae296bca67d2151160276 Mon Sep 17 00:00:00 2001 From: Paul Jolly Date: Mon, 18 Jan 2021 13:20:44 +0000 Subject: [PATCH] cmd/testscript: small tidy up (#120) Tidy up the naming of the directories within the temporary work dir, from an integer corresponding to the 1-indexed argument number to the basename of the argument. As part of this change we better assert that scripts are indeed left behind when -work is supplied. Also make consistent the way we render a filename argument in the case of an error. --- cmd/testscript/main.go | 47 ++++++++++++++++++++++---------- cmd/testscript/main_test.go | 20 ++++++++++++++ cmd/testscript/testdata/work.txt | 17 ++++++++++-- 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/cmd/testscript/main.go b/cmd/testscript/main.go index 28099ca9..8c3872f4 100644 --- a/cmd/testscript/main.go +++ b/cmd/testscript/main.go @@ -12,7 +12,6 @@ import ( "os" "os/exec" "path/filepath" - "strconv" "strings" "github.com/rogpeppe/go-internal/goproxytest" @@ -85,14 +84,26 @@ func mainerr() (retErr error) { files = []string{"-"} } - for i, fileName := range files { + dirNames := make(map[string]int) + for _, filename := range files { // TODO make running files concurrent by default? If we do, note we'll need to do // something smarter with the runner stdout and stderr below - runDir := filepath.Join(td, strconv.Itoa(i)) + + // Derive a name for the directory from the basename of file, making + // uniq by adding a numeric suffix in the case we otherwise end + // up with multiple files with the same basename + dirName := filepath.Base(filename) + count := dirNames[dirName] + dirNames[dirName] = count + 1 + if count != 0 { + dirName = fmt.Sprintf("%s%d", dirName, count) + } + + runDir := filepath.Join(td, dirName) if err := os.Mkdir(runDir, 0777); err != nil { - return fmt.Errorf("failed to create a run directory within %v for %v: %v", td, fileName, err) + return fmt.Errorf("failed to create a run directory within %v for %v: %v", td, renderFilename(filename), err) } - if err := run(runDir, fileName, *fVerbose, envVars.vals); err != nil { + if err := run(runDir, filename, *fVerbose, envVars.vals); err != nil { return err } } @@ -142,7 +153,16 @@ func (r runner) Verbose() bool { return r.verbose } -func run(runDir, fileName string, verbose bool, envVars []string) error { +// renderFilename renders filename in error messages, taking into account +// the filename could be the special "-" (stdin) +func renderFilename(filename string) string { + if filename == "-" { + return "" + } + return filename +} + +func run(runDir, filename string, verbose bool, envVars []string) error { var ar *txtar.Archive var err error @@ -152,19 +172,18 @@ func run(runDir, fileName string, verbose bool, envVars []string) error { return fmt.Errorf("failed to create goModProxy dir: %v", err) } - if fileName == "-" { - fileName = "" + if filename == "-" { byts, err := ioutil.ReadAll(os.Stdin) if err != nil { return fmt.Errorf("failed to read from stdin: %v", err) } ar = txtar.Parse(byts) } else { - ar, err = txtar.ParseFile(fileName) + ar, err = txtar.ParseFile(filename) } if err != nil { - return fmt.Errorf("failed to txtar parse %v: %v", fileName, err) + return fmt.Errorf("failed to txtar parse %v: %v", renderFilename(filename), err) } var script, gomodProxy txtar.Archive @@ -186,7 +205,7 @@ func run(runDir, fileName string, verbose bool, envVars []string) error { } if err := ioutil.WriteFile(filepath.Join(runDir, "script.txt"), txtar.Format(&script), 0666); err != nil { - return fmt.Errorf("failed to write script for %v: %v", fileName, err) + return fmt.Errorf("failed to write script for %v: %v", filename, err) } p := testscript.Params{ @@ -195,7 +214,7 @@ func run(runDir, fileName string, verbose bool, envVars []string) error { if _, err := exec.LookPath("go"); err == nil { if err := gotooltest.Setup(&p); err != nil { - return fmt.Errorf("failed to setup go tool for %v run: %v", fileName, err) + return fmt.Errorf("failed to setup go tool for %v run: %v", renderFilename(filename), err) } } @@ -214,7 +233,7 @@ func run(runDir, fileName string, verbose bool, envVars []string) error { if len(gomodProxy.Files) > 0 { srv, err := goproxytest.NewServer(mods, "") if err != nil { - return fmt.Errorf("cannot start proxy for %v: %v", fileName, err) + return fmt.Errorf("cannot start proxy for %v: %v", renderFilename(filename), err) } defer srv.Close() @@ -260,7 +279,7 @@ func run(runDir, fileName string, verbose bool, envVars []string) error { }() if err != nil { - return fmt.Errorf("error running %v in %v\n", fileName, runDir) + return fmt.Errorf("error running %v in %v\n", renderFilename(filename), runDir) } return nil diff --git a/cmd/testscript/main_test.go b/cmd/testscript/main_test.go index 48f7969d..ac7f63fb 100644 --- a/cmd/testscript/main_test.go +++ b/cmd/testscript/main_test.go @@ -54,6 +54,7 @@ func TestScripts(t *testing.T) { Cmds: map[string]func(ts *testscript.TestScript, neg bool, args []string){ "dropgofrompath": dropgofrompath, "setfilegoproxy": setfilegoproxy, + "expandone": expandone, }, } if err := gotooltest.Setup(&p); err != nil { @@ -94,3 +95,22 @@ func setfilegoproxy(ts *testscript.TestScript, neg bool, args []string) { } ts.Setenv("GOPROXY", "file://"+path) } + +// expandone takes a single glob-style argument that should expand to +// a single file, otherwise the command fails +func expandone(ts *testscript.TestScript, neg bool, args []string) { + if len(args) != 1 { + ts.Fatalf("expandone: expected a single argument") + } + if neg { + ts.Fatalf("unsupported: ! expandone") + } + glob := ts.MkAbs(args[0]) + matches, err := filepath.Glob(glob) + if err != nil { + ts.Fatalf("expandone: failed to glob %q: %v", glob, err) + } + if n := len(matches); n != 1 { + ts.Fatalf("expandone: %q matched %v files, not 1", glob, n) + } +} diff --git a/cmd/testscript/testdata/work.txt b/cmd/testscript/testdata/work.txt index 0e0afc99..1a185918 100644 --- a/cmd/testscript/testdata/work.txt +++ b/cmd/testscript/testdata/work.txt @@ -1,8 +1,19 @@ -# should support skip -unquote file.txt +# Test that passing -work leaves behind the working directory +# that contains the temporary directories within which the +# script arguments are expanded. +# +# This test also covers the use of multiple scripts which share +# the same basename, ensuring that the naming of the directories +# within the working directory. -testscript -v -work file.txt +unquote file.txt dir/file.txt + +testscript -v -work file.txt dir/file.txt stderr '\Qtemporary work directory: '$WORK'\E[/\\]tmp[/\\]' +expandone $WORK/tmp/testscript*/file.txt/script.txt +expandone $WORK/tmp/testscript*/file.txt1/script.txt -- file.txt -- >exec true +-- dir/file.txt -- +>exec true