Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: add support for txtar extension and prefer it #159

Merged
merged 1 commit into from Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
Expand Up @@ -7,3 +7,4 @@
# example, we want to allow a .txt file to contain a CRLF line ending
*.go text eol=lf
*.txt text eol=lf
*.txtar text eol=lf
8 changes: 4 additions & 4 deletions cmd/testscript/README.md
Expand Up @@ -37,7 +37,7 @@ script, and does not remove that directory when testscript exits.
Examples
========
The following example, fruit.txt, shows a simple reproduction that includes
The following example, fruit.txtar, shows a simple reproduction that includes
.gomodproxy supporting files:
go get -m fruit.com
Expand All @@ -58,7 +58,7 @@ The following example, fruit.txt, shows a simple reproduction that includes
const Name = "Apple"
Running testscript -v fruit.txt we get:
Running testscript -v fruit.txtar we get:
...
> go get -m fruit.com
Expand All @@ -76,7 +76,7 @@ Running testscript -v fruit.txt we get:
PASS
The following example, goimports.txt, shows a simple reproduction involving
The following example, goimports.txtar, shows a simple reproduction involving
goimports:
go install golang.org/x/tools/cmd/goimports
Expand All @@ -95,7 +95,7 @@ goimports:
const Pi = math.Pi
Running testscript -v goimports.txt we get:
Running testscript -v goimports.txtar we get:
...
> go install golang.org/x/tools/cmd/goimports
Expand Down
8 changes: 4 additions & 4 deletions cmd/testscript/help.go
Expand Up @@ -48,7 +48,7 @@ script, and does not remove that directory when testscript exits.
Examples
========

The following example, fruit.txt, shows a simple reproduction that includes
The following example, fruit.txtar, shows a simple reproduction that includes
.gomodproxy supporting files:

go get -m fruit.com
Expand All @@ -69,7 +69,7 @@ The following example, fruit.txt, shows a simple reproduction that includes

const Name = "Apple"

Running testscript -v fruit.txt we get:
Running testscript -v fruit.txtar we get:

...
> go get -m fruit.com
Expand All @@ -87,7 +87,7 @@ Running testscript -v fruit.txt we get:
PASS


The following example, goimports.txt, shows a simple reproduction involving
The following example, goimports.txtar, shows a simple reproduction involving
goimports:

go install golang.org/x/tools/cmd/goimports
Expand All @@ -106,7 +106,7 @@ goimports:

const Pi = math.Pi

Running testscript -v goimports.txt we get:
Running testscript -v goimports.txtar we get:

...
> go install golang.org/x/tools/cmd/goimports
Expand Down
2 changes: 1 addition & 1 deletion cmd/testscript/main.go
Expand Up @@ -196,7 +196,7 @@ func (tr *testRunner) run(runDir, filename string) error {
return fmt.Errorf("failed to write .gomodproxy files: %v", err)
}

scriptFile := filepath.Join(runDir, "script.txt")
scriptFile := filepath.Join(runDir, "script.txtar")

if err := ioutil.WriteFile(scriptFile, txtar.Format(&script), 0666); err != nil {
return fmt.Errorf("failed to write script for %v: %v", renderFilename(filename), err)
Expand Down
4 changes: 2 additions & 2 deletions cmd/testscript/testdata/work.txt
Expand Up @@ -12,8 +12,8 @@ testscript -v -work file.txt dir/file.txt
stderr '^temporary work directory: \Q'$WORK'\E[/\\]\.tmp[/\\]'
stderr '^temporary work directory for file.txt: \Q'$WORK'\E[/\\]\.tmp[/\\]'
stderr '^temporary work directory for dir[/\\]file.txt: \Q'$WORK'\E[/\\]\.tmp[/\\]'
expandone $WORK/.tmp/testscript*/file.txt/script.txt
expandone $WORK/.tmp/testscript*/file.txt1/script.txt
expandone $WORK/.tmp/testscript*/file.txt/script.txtar
expandone $WORK/.tmp/testscript*/file.txt1/script.txtar

-- file.txt --
>exec true
Expand Down
2 changes: 1 addition & 1 deletion cmd/txtar-addmod/addmod.go
Expand Up @@ -202,7 +202,7 @@ func main1() int {
break
}
} else {
if err := ioutil.WriteFile(filepath.Join(targetDir, modDir+".txt"), data, 0666); err != nil {
if err := ioutil.WriteFile(filepath.Join(targetDir, modDir+".txtar"), data, 0666); err != nil {
log.Printf("%s: %v", arg, err)
exitCode = 1
continue
Expand Down
2 changes: 1 addition & 1 deletion cmd/txtar-addmod/testdata/encode.txt
Expand Up @@ -2,4 +2,4 @@ mkdir $WORK/out
txtar-addmod $WORK/out github.com/shurcooL/httpfs@v0.0.0-20171119174359-809beceb2371
! stdout .+
! stderr .+
exists $WORK/out/github.com_shurcoo!l_httpfs_v0.0.0-20171119174359-809beceb2371.txt
exists $WORK/out/github.com_shurcoo!l_httpfs_v0.0.0-20171119174359-809beceb2371.txtar
6 changes: 3 additions & 3 deletions cmd/txtar-addmod/testdata/txtar-addmod-self.txt
Expand Up @@ -2,8 +2,8 @@ mkdir $WORK/out
txtar-addmod $WORK/out github.com/gobin-testrepos/simple-main
! stdout .+
! stderr .+
exists $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txt
! grep foobar $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txt
exists $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txtar
! grep foobar $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txtar

txtar-addmod -all $WORK/out github.com/gobin-testrepos/simple-main
grep '-- foobar --' $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txt
grep '-- foobar --' $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txtar
4 changes: 2 additions & 2 deletions cmd/txtar-c/savedir.go
Expand Up @@ -6,7 +6,7 @@
//
// Usage:
//
// txtar-c /path/to/dir >saved.txt
// txtar-c /path/to/dir >saved.txtar
//
// See https://godoc.org/github.com/rogpeppe/go-internal/txtar for details of the format
// and how to parse a txtar file.
Expand All @@ -30,7 +30,7 @@ import (
var flag = stdflag.NewFlagSet(os.Args[0], stdflag.ContinueOnError)

func usage() {
fmt.Fprintf(os.Stderr, "usage: txtar-c dir >saved.txt\n")
fmt.Fprintf(os.Stderr, "usage: txtar-c dir >saved.txtar\n")
flag.PrintDefaults()
}

Expand Down
29 changes: 19 additions & 10 deletions goproxytest/proxy.go
Expand Up @@ -7,9 +7,9 @@ Package goproxytest serves Go modules from a proxy server designed to run on
localhost during tests, both to make tests avoid requiring specific network
servers and also to make them significantly faster.
Each module archive is either a file named path_vers.txt or a directory named
path_vers, where slashes in path have been replaced with underscores. The
archive or directory must contain two files ".info" and ".mod", to be served as
Each module archive is either a file named path_vers.txtar or path_vers.txt, or
a directory named path_vers, where slashes in path have been replaced with underscores.
The archive or directory must contain two files ".info" and ".mod", to be served as
the info and mod files in the proxy protocol (see
https://research.swtch.com/vgo-module). The remaining files are served as the
content of the module zip file. The path@vers prefix required of files in the
Expand Down Expand Up @@ -96,10 +96,15 @@ func (srv *Server) readModList() error {
}
for _, info := range infos {
name := info.Name()
if !strings.HasSuffix(name, ".txt") && !info.IsDir() {
switch {
case strings.HasSuffix(name, ".txt"):
name = strings.TrimSuffix(name, ".txt")
case strings.HasSuffix(name, ".txtar"):
name = strings.TrimSuffix(name, ".txtar")
case info.IsDir():
Copy link
Owner

Choose a reason for hiding this comment

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

Why are we allowing a directory when we weren't before? Maybe worth a comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we did continue the loop in the case of info.IsDir() before; see the deleted line. I essentially inverted the !foo && !bar with a break into a foo || bar with an else (the default case) which breaks.

one logical difference is that the old code would accept module_version.txt/ as a directory, happily stripping the .txt suffix. I think that was a coding mistake, because that feature was never documented - the docs only refer to module_version.txt as a file, and module_version/ as a directory.

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry, I misread the previous logic. All LGTM.

default:
continue
}
name = strings.TrimSuffix(name, ".txt")
i := strings.LastIndex(name, "_v")
if i < 0 {
continue
Expand Down Expand Up @@ -281,13 +286,17 @@ func (srv *Server) readArchive(path, vers string) *txtar.Archive {
}

prefix := strings.Replace(enc, "/", "_", -1)
name := filepath.Join(srv.dir, prefix+"_"+encVers+".txt")
name := filepath.Join(srv.dir, prefix+"_"+encVers)
txtName := name + ".txt"
txtarName := name + ".txtar"
a := srv.archiveCache.Do(name, func() interface{} {
a, err := txtar.ParseFile(name)
a, err := txtar.ParseFile(txtarName)
if os.IsNotExist(err) {
// we fallback to trying a directory
name = strings.TrimSuffix(name, ".txt")

// fall back to trying with the .txt extension
a, err = txtar.ParseFile(txtName)
}
if os.IsNotExist(err) {
// fall back to trying a directory
a = new(txtar.Archive)

err = filepath.Walk(name, func(path string, info os.FileInfo, err error) error {
Expand Down
6 changes: 3 additions & 3 deletions testscript/doc.go
Expand Up @@ -14,10 +14,10 @@ To invoke the tests, call testscript.Run. For example:
})
}
A testscript directory holds test scripts *.txt run during 'go test'.
A testscript directory holds test scripts with extension txtar or txt run during 'go test'.
Each script defines a subtest; the exact set of allowable commands in a
script are defined by the parameters passed to the Run function.
To run a specific script foo.txt
To run a specific script foo.txtar or foo.txt, run
go test cmd/go -run=TestName/^foo$
Expand Down Expand Up @@ -68,7 +68,7 @@ systems, ".exe" on Windows.
The script's supporting files are unpacked relative to $WORK
and then the script begins execution in that
directory as well. Thus the example above runs in $WORK
with $WORK/hello.txt containing the listed contents.
with $WORK/hello.txtar containing the listed contents.
The lines at the top of the script are a sequence of commands to be
executed by a small script engine in the testscript package (not the system
Expand Down
6 changes: 6 additions & 0 deletions testscript/testdata/testscript_update_script.txt
@@ -1,7 +1,13 @@
# Check that we support both txt and txtar extensions.

unquote scripts/testscript.txt
unquote testscript-new.txt

cp scripts/testscript.txt scripts/testscript2.txtar

testscript -update scripts
cmp scripts/testscript.txt testscript-new.txt
cmp scripts/testscript2.txtar testscript-new.txt

-- scripts/testscript.txt --
>fprintargs stdout right
Expand Down
25 changes: 17 additions & 8 deletions testscript/testscript.go
Expand Up @@ -108,8 +108,8 @@ func (e *Env) T() T {
// Params holds parameters for a call to Run.
type Params struct {
// Dir holds the name of the directory holding the scripts.
// All files in the directory with a .txt suffix will be considered
// as test scripts. By default the current directory is used.
// All files in the directory with a .txtar or .txt suffix will be
// considered as test scripts. By default the current directory is used.
// Dir is interpreted relative to the current test directory.
Dir string

Expand Down Expand Up @@ -158,7 +158,7 @@ type Params struct {
}

// RunDir runs the tests in the given directory. All files in dir with a ".txt"
// are considered to be test files.
// or ".txtar" extension are considered to be test files.
func Run(t *testing.T, p Params) {
RunT(tshim{t}, p)
}
Expand Down Expand Up @@ -200,13 +200,22 @@ func (t tshim) Verbose() bool {
// RunT is like Run but uses an interface type instead of the concrete *testing.T
// type to make it possible to use testscript functionality outside of go test.
func RunT(t T, p Params) {
glob := filepath.Join(p.Dir, "*.txt")
files, err := filepath.Glob(glob)
if err != nil {
entries, err := os.ReadDir(p.Dir)
if os.IsNotExist(err) {
// Continue so we give a helpful error on len(files)==0 below.
} else if err != nil {
t.Fatal(err)
}
var files []string
for _, entry := range entries {
name := entry.Name()
if strings.HasSuffix(name, ".txtar") || strings.HasSuffix(name, ".txt") {
files = append(files, filepath.Join(p.Dir, name))
}
}

if len(files) == 0 {
t.Fatal(fmt.Sprintf("no scripts found matching glob: %v", glob))
t.Fatal(fmt.Sprintf("no txtar nor txt scripts found in dir %s", p.Dir))
}
testTempDir := p.WorkdirRoot
if testTempDir == "" {
Expand Down Expand Up @@ -814,7 +823,7 @@ func (ts *TestScript) Getenv(key string) string {
// parse parses a single line as a list of space-separated arguments
// subject to environment variable expansion (but not resplitting).
// Single quotes around text disable splitting and expansion.
// To embed a single quote, double it: 'Don''t communicate by sharing memory.'
// To embed a single quote, double it: 'Dont communicate by sharing memory.'
func (ts *TestScript) parse(line string) []string {
ts.line = line

Expand Down
2 changes: 1 addition & 1 deletion testscript/testscript_test.go
Expand Up @@ -312,7 +312,7 @@ func TestBadDir(t *testing.T) {
if got := len(ft.failMsgs); got != wantCount {
t.Fatalf("expected %v fail message; got %v", wantCount, got)
}
wantMsg := regexp.MustCompile(`no scripts found matching glob: thiswillnevermatch[/\\]\*\.txt`)
wantMsg := regexp.MustCompile(`no txtar nor txt scripts found in dir thiswillnevermatch`)
if got := ft.failMsgs[0]; !wantMsg.MatchString(got) {
t.Fatalf("expected msg to match `%v`; got:\n%v", wantMsg, got)
}
Expand Down