Skip to content

WIP: Install and use source-map-support module for tests in Circle CI. #449

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

WIP: Install and use source-map-support module for tests in Circle CI. #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aee41f9
circle: Require source-map-support for tests.
dmitshur Apr 30, 2016
2aeca02
circle: Update to Node 6.0.0.
dmitshur Apr 30, 2016
16ec727
circle: Set NODE_PATH env var.
dmitshur Apr 30, 2016
ad23587
circle: Set correct global node_module path.
dmitshur Apr 30, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions circle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
machine:
node:
version: 5.1.0
environment:
SOURCE_MAP_SUPPORT: false
version: 6.0.0

dependencies:
pre:
- cd /usr/local && sudo rm -rf go && curl https://storage.googleapis.com/golang/go1.6.2.linux-amd64.tar.gz | sudo tar -xz && sudo chmod a+w go/src/path/filepath
post:
- mv ./gopherjs $HOME/bin
- npm install --global source-map-support
- npm install --global node-gyp
- cd node-syscall && node-gyp rebuild && mkdir -p ~/.node_libraries/ && cp build/Release/syscall.node ~/.node_libraries/syscall.node

Expand All @@ -17,5 +16,5 @@ test:
- diff -u <(echo -n) <(gofmt -d .)
- go tool vet *.go # Go package in root directory.
- for d in */; do echo $d; done | grep -v tests/ | grep -v third_party/ | xargs go tool vet # All subdirectories except "tests", "third_party".
- gopherjs test --short --minify github.com/gopherjs/gopherjs/tests github.com/gopherjs/gopherjs/tests/main github.com/gopherjs/gopherjs/js archive/tar archive/zip bufio bytes compress/bzip2 compress/flate compress/gzip compress/lzw compress/zlib container/heap container/list container/ring crypto/aes crypto/cipher crypto/des crypto/dsa crypto/ecdsa crypto/elliptic crypto/hmac crypto/md5 crypto/rand crypto/rc4 crypto/rsa crypto/sha1 crypto/sha256 crypto/sha512 crypto/subtle crypto/x509 database/sql database/sql/driver debug/dwarf debug/elf debug/macho debug/pe encoding/ascii85 encoding/asn1 encoding/base32 encoding/base64 encoding/binary encoding/csv encoding/gob encoding/hex encoding/json encoding/pem encoding/xml errors expvar flag fmt go/ast go/constant go/doc go/format go/parser go/printer go/scanner go/token hash/adler32 hash/crc32 hash/crc64 hash/fnv html html/template image image/color image/draw image/gif image/jpeg image/png index/suffixarray io io/ioutil math math/big math/cmplx math/rand mime mime/multipart mime/quotedprintable net/http/cookiejar net/http/fcgi net/mail net/rpc/jsonrpc net/textproto net/url path path/filepath reflect regexp regexp/syntax sort strconv strings sync sync/atomic testing/quick text/scanner text/tabwriter text/template text/template/parse time unicode unicode/utf16 unicode/utf8
- go test -v -race ./...
- NODE_PATH=$(npm root --global) gopherjs test --short --minify github.com/gopherjs/gopherjs/tests github.com/gopherjs/gopherjs/tests/main github.com/gopherjs/gopherjs/js archive/tar archive/zip bufio bytes compress/bzip2 compress/flate compress/gzip compress/lzw compress/zlib container/heap container/list container/ring crypto/aes crypto/cipher crypto/des crypto/dsa crypto/ecdsa crypto/elliptic crypto/hmac crypto/md5 crypto/rand crypto/rc4 crypto/rsa crypto/sha1 crypto/sha256 crypto/sha512 crypto/subtle crypto/x509 database/sql database/sql/driver debug/dwarf debug/elf debug/macho debug/pe encoding/ascii85 encoding/asn1 encoding/base32 encoding/base64 encoding/binary encoding/csv encoding/gob encoding/hex encoding/json encoding/pem encoding/xml errors expvar flag fmt go/ast go/constant go/doc go/format go/parser go/printer go/scanner go/token hash/adler32 hash/crc32 hash/crc64 hash/fnv html html/template image image/color image/draw image/gif image/jpeg image/png index/suffixarray io io/ioutil math math/big math/cmplx math/rand mime mime/multipart mime/quotedprintable net/http/cookiejar net/http/fcgi net/mail net/rpc/jsonrpc net/textproto net/url path path/filepath reflect regexp regexp/syntax sort strconv strings sync sync/atomic testing/quick text/scanner text/tabwriter text/template text/template/parse time unicode unicode/utf16 unicode/utf8
- NODE_PATH=$(npm root --global) go test -v -race ./...
8 changes: 8 additions & 0 deletions tests/gorepo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,19 @@ package tests_test
import (
"os"
"os/exec"
"strconv"
"testing"
)

// Go repository basic compiler tests, and regression tests for fixed compiler bugs.
func TestGoRepositoryCompilerTests(t *testing.T) {
if b, _ := strconv.ParseBool(os.Getenv("SOURCE_MAP_SUPPORT")); os.Getenv("SOURCE_MAP_SUPPORT") != "" && !b {
t.Fatal("Source maps disabled, but required for this test. Use SOURCE_MAP_SUPPORT=true or unset it completely.")
Copy link
Member

Choose a reason for hiding this comment

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

t.Skip?

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered skip, but was worried it might be bad to simply skip such an important test so easily.

Hmm...

Maybe skip if short flag is given?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe skip if short flag is given?

I don't think that would be good, better be consistent.

I think I've seen Skip in the core packages for this situation. Reasoning might go like this:
Fail -> Implementation broken
Skip -> Environment broken/insufficient

Copy link
Member Author

@dmitshur dmitshur May 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I can change to skip, sure, I just don't want us to get into a situation where those tests are always skipped and we don't notice/realize.

Copy link
Member Author

@dmitshur dmitshur May 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I've looked in https://github.com/golang/go/blob/master/src/cmd/go/go_test.go to see when skip is used.

It's used to skip individual specific tests when environment is insufficient. For example, TestCgoShowsFullPathNames is skipped when cgo is not available.

The difference is that TestGoRepositoryCompilerTests runs many, many tests. Only one of those actually depends on source-map-support to be enabled. Skipping all of TestGoRepositoryCompilerTests just because source-map-support is equivalen to skipping all of cmd/go tests if cgo is not enabled. I don't think it's acceptable.

Let's find a better solution...

Copy link
Member Author

@dmitshur dmitshur May 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

In the PR description, I asked:

What do you think about requiring source-map-support for tests? Right now they're disabled in Circle CI. Enabling it fixes one failing test from Go compiler test suite (fixedbugs/issue5856.go).

Maybe it's not worth it to require source-map-support to be mandatory for tests?

Copy link
Member

Choose a reason for hiding this comment

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

I think the most important aspect is that the CI system tests properly. Local testing has a much lower priority and I would be okay with skipping tests in that situation. If people download the source code, run the tests and they fail, then they will probably think that the tests are broken.

Copy link
Member Author

@dmitshur dmitshur May 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I'll find an acceptable solution and update this PR; let's not merge it until then.

I've already cherry-picked the acceptable individual commits out from here.

}
if err := exec.Command("node", "--require", "source-map-support/register", "--eval", "").Run(); err != nil {
t.Fatal("Source maps disabled, but required for this test. Use Node.js 4.x with source-map-support module for nice stack traces.")
}

args := []string{"go", "run", "run.go", "-summary"}
if testing.Verbose() {
args = append(args, "-v")
Expand Down
1 change: 0 additions & 1 deletion tests/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ var knownFails = map[string]failReason{
"fixedbugs/issue4388.go": {desc: "Error: expected <autogenerated>:1 have anonymous function:0"},
"fixedbugs/issue4562.go": {desc: "Error: cannot find issue4562.go on stack"},
"fixedbugs/issue4620.go": {desc: "map[0:1 1:2], Error: m[i] != 2"},
"fixedbugs/issue5856.go": {desc: "BUG: defer called from /private/var/folders/tw/kgz4v2kn4n7d7ryg5k_z3dk40000gn/T/issue5856.go.931062983:15135, want issue5856.go:28"},
"fixedbugs/issue6899.go": {desc: "incorrect output -0"},
"fixedbugs/issue7550.go": {desc: "FATAL ERROR: invalid table size Allocation failed - process out of memory"},
"fixedbugs/issue7690.go": {desc: "Error: runtime error: slice bounds out of range"},
Expand Down