Skip to content

Commit

Permalink
internal/cuetest: drive long tests via an env var
Browse files Browse the repository at this point in the history
Using a build tag like "go test -tags=long" works,
but has two noticeable disadvantages:

1. Flipping a build tag in a dependency package means rebuilding it
   along all downstream packages. Rebuilding a big portion of the
   packages in the CUE module feels unnecessary to just flip a boolean.

2. Using build tags to skip tests is generally discouraged, as it is
   possible to break their compilation and not notice given that the
   Go files aren't built at all. We shouldn't set the precedent.

The long tests are now run via a non-empty env var:

	CUE_LONG=true go test ./...

Finally, update the CI setup to use the new mechanism.

Change-Id: I589e7c5163d9d763df3584aa16f7eef3d5e32c81
Signed-off-by: Daniel Martí <[email protected]>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/536306
Unity-Result: CUEcueckoo <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Paul Jolly <[email protected]>
  • Loading branch information
mvdan committed Apr 24, 2022
1 parent f207b0b commit 33b18be
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 101 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ jobs:
key: ${{ runner.os }}-${{ matrix.go-version }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: ${{ runner.os }}-${{ matrix.go-version }}-go-
- if: ${{ github.ref == 'refs/heads/master' }}
name: Set go build tags
run: go env -w GOFLAGS=-tags=long
run: echo CUE_LONG=true >> $GITHUB_ENV
- if: matrix.go-version == '1.16.x' && matrix.os == 'ubuntu-18.04'
name: Generate
run: go generate ./...
Expand Down
14 changes: 4 additions & 10 deletions cmd/cue/cmd/testdata/script/cmd_github.txt
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,7 @@ jobs:
key: ${{ runner.os }}-${{ matrix.go-version }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: ${{ runner.os }}-${{ matrix.go-version }}-go-
- if: ${{ github.ref == 'refs/heads/master' }}
name: Set go build tags
run: go env -w GOFLAGS=-tags=long
run: echo CUE_LONG=true >> $GITHUB_ENV
- if: matrix.go-version == '1.16.x' && matrix.os == 'ubuntu-18.04'
name: Generate
run: go generate ./...
Expand Down Expand Up @@ -1123,9 +1122,9 @@ test: _#bashWorkflow & {
needs: "start"
strategy: _#testStrategy
"runs-on": "${{ matrix.os }}"
steps: [_#writeNetrcFile, _#installGo, _#checkoutCode, _#cacheGoModules, _#setGoBuildTags & {
_#tags: "long"
if: "${{ \(_#isMaster) }}"
steps: [_#writeNetrcFile, _#installGo, _#checkoutCode, _#cacheGoModules, _#step & {
if: "${{ \(_#isMaster) }}"
run: "echo CUE_LONG=true >> $GITHUB_ENV"
}, _#goGenerate, _#goTest, _#goTestRace & {
if: "${{ matrix.go-version == '\(_#latestStableGo)' && matrix.os == '\(_#linuxMachine)' }}"
}, _#goReleaseCheck, _#checkGitClean, _#pullThroughProxy, _#failCLBuild]
Expand Down Expand Up @@ -1342,11 +1341,6 @@ _#testStrategy: {
os: [_#linuxMachine, _#macosMachine, _#windowsMachine]
}
}
_#setGoBuildTags: _#step & {
_#tags: string
name: "Set go build tags"
run: "go env -w GOFLAGS=-tags=\(_#tags)"
}
_#installGo: _#step & {
name: "Install Go"
uses: "actions/setup-go@v3"
Expand Down
37 changes: 7 additions & 30 deletions cue/testdata/eval/github.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ test: _#bashWorkflow & {
needs: "start"
strategy: _#testStrategy
"runs-on": "${{ matrix.os }}"
steps: [_#writeNetrcFile, _#installGo, _#checkoutCode, _#cacheGoModules, _#setGoBuildTags & {
_#tags: "long"
if: "${{ \(_#isMaster) }}"
steps: [_#writeNetrcFile, _#installGo, _#checkoutCode, _#cacheGoModules, _#step & {
if: "${{ \(_#isMaster) }}"
run: "echo CUE_LONG=true >> $GITHUB_ENV"
}, _#goGenerate, _#goTest, _#goTestRace & {
if: "${{ matrix.go-version == '\(_#latestStableGo)' && matrix.os == '\(_#linuxMachine)' }}"
}, _#goReleaseCheck, _#checkGitClean, _#pullThroughProxy, _#failCLBuild]
Expand Down Expand Up @@ -274,11 +274,6 @@ _#testStrategy: {
os: [_#linuxMachine, _#macosMachine, _#windowsMachine]
}
}
_#setGoBuildTags: _#step & {
_#tags: string
name: "Set go build tags"
run: "go env -w GOFLAGS=-tags=\(_#tags)"
}
_#installGo: _#step & {
name: "Install Go"
uses: "actions/setup-go@v3"
Expand Down Expand Up @@ -1121,10 +1116,8 @@ import "strings"
}
}
4: (#struct){
_#tags(:ci): (string){ "long" }
name: (string){ "Set go build tags" }
run: (string){ "go env -w GOFLAGS=-tags=long" }
if: (string){ "${{ github.ref == 'refs/heads/master' }}" }
run: (string){ "echo CUE_LONG=true >> $GITHUB_ENV" }
}
5: (#struct){
name: (string){ "Generate" }
Expand Down Expand Up @@ -1639,10 +1632,8 @@ import "strings"
}
}
4: (#struct){
_#tags(:ci): (string){ "long" }
name: (string){ "Set go build tags" }
run: (string){ "go env -w GOFLAGS=-tags=long" }
if: (string){ "${{ github.ref == 'refs/heads/master' }}" }
run: (string){ "echo CUE_LONG=true >> $GITHUB_ENV" }
}
5: (#struct){
name: (string){ "Generate" }
Expand Down Expand Up @@ -2130,15 +2121,6 @@ import "strings"
}
}
}
_#setGoBuildTags(:ci): (#struct){
_#tags(:ci): (string){ string }
name: (string){ "Set go build tags" }
run: (_|_){
// [incomplete] _#setGoBuildTags.run: invalid interpolation: non-concrete value string (type string):
// ./workflows.cue:270:10
// ./workflows.cue:268:10
}
}
_#installGo(:ci): (#struct){
name: (string){ "Install Go" }
uses: (string){ "actions/setup-go@v3" }
Expand Down Expand Up @@ -2251,9 +2233,9 @@ import "strings"
〈4;_#installGo〉,
〈4;_#checkoutCode〉,
〈4;_#cacheGoModules〉,
(〈4;_#setGoBuildTags〉 & {
_#tags: "long"
(〈4;_#step〉 & {
if: "${{ \(〈4;_#isMaster〉) }}"
run: "echo CUE_LONG=true >> $GITHUB_ENV"
}),
〈4;_#goGenerate〉,
〈4;_#goTest〉,
Expand Down Expand Up @@ -2515,11 +2497,6 @@ import "strings"
]
}
}
_#setGoBuildTags: (〈0;_#step〉 & {
_#tags: string
name: "Set go build tags"
run: "go env -w GOFLAGS=-tags=\(〈0;_#tags〉)"
})
_#installGo: (〈0;_#step〉 & {
name: "Install Go"
uses: "actions/setup-go@v3"
Expand Down
14 changes: 3 additions & 11 deletions internal/ci/workflows.cue
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ test: _#bashWorkflow & {
_#installGo,
_#checkoutCode,
_#cacheGoModules,
_#setGoBuildTags & {
_#tags: "long"
if: "${{ \(_#isMaster) }}"
_#step & {
if: "${{ \(_#isMaster) }}"
run: "echo CUE_LONG=true >> $GITHUB_ENV"
},
_#goGenerate,
_#goTest,
Expand Down Expand Up @@ -353,14 +353,6 @@ _#testStrategy: {
}
}

_#setGoBuildTags: _#step & {
_#tags: string
name: "Set go build tags"
run: """
go env -w GOFLAGS=-tags=\(_#tags)
"""
}

_#installGo: _#step & {
name: "Install Go"
uses: "actions/setup-go@v3"
Expand Down
14 changes: 10 additions & 4 deletions internal/cuetest/cuetest.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import (
)

const (
// envUpdate is used in the definition of UpdateGoldenFiles
envLong = "CUE_LONG"

envUpdate = "CUE_UPDATE"

// envNonIssues can be set to a regular expression which indicates what
Expand All @@ -51,10 +52,15 @@ var (
}
)

// Long determines whether long tests should be run.
// It is controlled by setting CUE_LONG to a non-empty string like "true".
// Note that it is not the equivalent of not supplying -short.
var Long = os.Getenv(envLong) != ""

// UpdateGoldenFiles determines whether testscript scripts should update txtar
// archives in the event of cmp failures. It corresponds to
// testscript.Params.UpdateGoldenFiles. See the docs for
// testscript.Params.UpdateGoldenFiles for more details.
// archives in the event of cmp failures.
// It is controlled by setting CUE_UPDATE to a non-empty string like "true".
// It corresponds to testscript.Params.UpdateGoldenFiles; see its docs for details.
var UpdateGoldenFiles = os.Getenv(envUpdate) != ""

// FormatTxtar ensures that .cue files in txtar test archives are well
Expand Down
22 changes: 0 additions & 22 deletions internal/cuetest/long.go

This file was deleted.

22 changes: 0 additions & 22 deletions internal/cuetest/nolong.go

This file was deleted.

0 comments on commit 33b18be

Please sign in to comment.