Skip to content

Commit

Permalink
avoid cross-package references in tests (google#385)
Browse files Browse the repository at this point in the history
The previous implementation of DataFile (which abstracts different build systems' ways of locating test data files) constructed file names of data files using $GOPATH; also, it used a cross-package reference to locate the assert.star file. This doesn't work under Go modules. Now, DataFile looks only in the current directory, and the assert.star file is moved into a constant section of the executable.

Also, fix tests historically broken on Mac/Windows due to diff and CR LF differences.
  • Loading branch information
adonovan authored Aug 31, 2021
1 parent d12bc23 commit 94adf23
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 10 deletions.
9 changes: 8 additions & 1 deletion internal/chunkedfile/chunkedfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"fmt"
"io/ioutil"
"regexp"
"runtime"
"strconv"
"strings"
)
Expand Down Expand Up @@ -61,7 +62,13 @@ func Read(filename string, report Reporter) (chunks []Chunk) {
return
}
linenum := 1
for i, chunk := range strings.Split(string(data), "\n---\n") {

eol := "\n"
if runtime.GOOS == "windows" {
eol = "\r\n"
}

for i, chunk := range strings.Split(string(data), eol+"---"+eol) {
if debug {
fmt.Printf("chunk %d at line %d: %s\n", i, linenum, chunk)
}
Expand Down
5 changes: 3 additions & 2 deletions internal/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ set -eu
cp go.mod go.mod.orig
cp go.sum go.sum.orig
go mod tidy
diff go.mod.orig go.mod
diff go.sum.orig go.sum
# Use -w to ignore differences in OS newlines.
diff -w go.mod.orig go.mod
diff -w go.sum.orig go.sum
rm go.mod.orig go.sum.orig

# Run tests
Expand Down
7 changes: 7 additions & 0 deletions starlarktest/assert.star → starlarktest/assert.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
package starlarktest

// assertStar contains the logical data file assert.star.
// TODO(adonovan): move it back into an actual file,
// when fs.Embed is more than two releases old.
const assertStar = `
# Predeclared built-ins for this module:
#
# error(msg): report an error in Go's test framework without halting execution.
Expand Down Expand Up @@ -49,3 +55,4 @@ assert = module(
contains = _contains,
fails = _fails,
)
`
12 changes: 5 additions & 7 deletions starlarktest/starlarktest.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ package starlarktest // import "go.starlark.net/starlarktest"

import (
"fmt"
"go/build"
"os"
"path/filepath"
"regexp"
Expand Down Expand Up @@ -68,12 +67,9 @@ func LoadAssertModule() (starlark.StringDict, error) {
"_freeze": starlark.NewBuiltin("freeze", freeze),
}
// TODO(adonovan): embed the file using embed.FS when we can rely on go1.16,
// and stop using DataFile. Otherwise this reaches from the application's working
// directory into the starlarktest package's directory, which is not compatible
// with modules.
filename := DataFile("starlarktest", "assert.star")
// and make the apparent filename reference that file.
thread := new(starlark.Thread)
assert, assertErr = starlark.ExecFile(thread, filename, nil, predeclared)
assert, assertErr = starlark.ExecFile(thread, "builtins/assert.star", assertStar, predeclared)
})
return assert, assertErr
}
Expand Down Expand Up @@ -147,5 +143,7 @@ var DataFile = func(pkgdir, filename string) string {
return filepath.Join(testSrcdir, "net_starlark_go", pkgdir, filename)
}

return filepath.Join(build.Default.GOPATH, "src/go.starlark.net", pkgdir, filename)
// Under go test, ignore pkgdir, which is the directory of the
// current package relative to the module root.
return filename
}

0 comments on commit 94adf23

Please sign in to comment.