Skip to content

Commit 8ed73c7

Browse files
dmitshurstapelberg
andcommitted
all: run integration test on longtest builders in CI
Remove the 'ignore' build constraint from integration_test.go and arrange things such that the integration test runs on longtest (but not longtest-race, for now anyway) builders. The existing scripts for running the invocation test locally are preserved and can be used as before. This change relies on the builders being configured to place pre-built protoc and conformance_test_runner binaries in $PATH (CL 547116); thanks to Michael for making them available via a CIPD package. Apply a few improvements to downloadFile and downloadArchive that came about from earlier prototypes that used them and uncovered some issues. Fixes golang/go#64066. Change-Id: I48b163e3ea9c441b748071da340d3e37282cc22b Co-authored-by: Michael Stapelberg <stapelberg@golang.org> Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/541123 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Stapelberg <stapelberg@google.com>
1 parent e8baad6 commit 8ed73c7

5 files changed

Lines changed: 89 additions & 26 deletions

File tree

.github/workflows/test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,4 @@ jobs:
2727
~/.cache/bazel
2828
key: ${{ runner.os }}-${{ hashFiles('integration_test.go') }}
2929
- name: Test
30-
run: go test -v -timeout=60m -count=1 -failfast integration_test.go
30+
run: go test -run='^TestIntegration$' -v -timeout=60m -count=1 -failfast "$@"

integration_test.go

Lines changed: 85 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build ignore
6-
// +build ignore
7-
85
package main
96

107
import (
@@ -16,13 +13,16 @@ import (
1613
"flag"
1714
"fmt"
1815
"io"
16+
"io/fs"
1917
"io/ioutil"
2018
"net/http"
2119
"os"
2220
"os/exec"
2321
"path/filepath"
22+
"reflect"
2423
"regexp"
2524
"runtime"
25+
"runtime/debug"
2626
"strings"
2727
"sync"
2828
"testing"
@@ -43,8 +43,8 @@ var (
4343
"1.17.13",
4444
"1.18.10",
4545
"1.19.13",
46-
"1.20.7",
47-
"1.21.1",
46+
"1.20.12",
47+
"1.21.5",
4848
}
4949
}()
5050
golangLatest = golangVersions[len(golangVersions)-1]
@@ -65,7 +65,25 @@ var (
6565
protobufPath string
6666
)
6767

68-
func Test(t *testing.T) {
68+
func TestIntegration(t *testing.T) {
69+
if testing.Short() {
70+
t.Skip("skipping integration test in short mode")
71+
}
72+
if os.Getenv("GO_BUILDER_NAME") != "" {
73+
// To start off, run on longtest builders, not longtest-race ones.
74+
if race() {
75+
t.Skip("skipping integration test in race mode on builders")
76+
}
77+
// When on a builder, run even if it's not explicitly requested
78+
// provided our caller isn't already running it.
79+
if os.Getenv("GO_PROTOBUF_INTEGRATION_TEST_RUNNING") == "1" {
80+
t.Skip("protobuf integration test is already running, skipping nested invocation")
81+
}
82+
os.Setenv("GO_PROTOBUF_INTEGRATION_TEST_RUNNING", "1")
83+
} else if flag.Lookup("test.run").Value.String() != "^TestIntegration$" {
84+
t.Skip("not running integration test if not explicitly requested via test.bash")
85+
}
86+
6987
mustInitDeps(t)
7088
mustHandleFlags(t)
7189

@@ -236,9 +254,7 @@ func mustInitDeps(t *testing.T) {
236254
}
237255
finishWork()
238256

239-
// Download and build the protobuf toolchain.
240-
// We avoid downloading the pre-compiled binaries since they do not contain
241-
// the conformance test runner.
257+
// Get the protobuf toolchain.
242258
protobufPath = startWork("protobuf-" + protobufVersion)
243259
if _, err := os.Stat(protobufPath); err != nil {
244260
fmt.Printf("download %v\n", filepath.Base(protobufPath))
@@ -250,16 +266,32 @@ func mustInitDeps(t *testing.T) {
250266
command{Dir: testDir}.mustRun(t, "git", "clone", "https://github.com/protocolbuffers/protobuf", "protobuf-"+protobufVersion)
251267
command{Dir: protobufPath}.mustRun(t, "git", "checkout", checkoutVersion)
252268

253-
fmt.Printf("build %v\n", filepath.Base(protobufPath))
254-
env := os.Environ()
255-
if runtime.GOOS == "darwin" {
256-
// Adding this environment variable appears to be necessary for macOS builds.
257-
env = append(env, "CC=clang")
269+
if os.Getenv("GO_BUILDER_NAME") != "" {
270+
// If this is running on the Go build infrastructure,
271+
// use pre-built versions of these binaries that the
272+
// builders are configured to provide in $PATH.
273+
protocPath, err := exec.LookPath("protoc")
274+
check(err)
275+
confTestRunnerPath, err := exec.LookPath("conformance_test_runner")
276+
check(err)
277+
check(os.MkdirAll(filepath.Join(protobufPath, "bazel-bin", "conformance"), 0775))
278+
check(os.Symlink(protocPath, filepath.Join(protobufPath, "bazel-bin", "protoc")))
279+
check(os.Symlink(confTestRunnerPath, filepath.Join(protobufPath, "bazel-bin", "conformance", "conformance_test_runner")))
280+
} else {
281+
// In other environments, download and build the protobuf toolchain.
282+
// We avoid downloading the pre-compiled binaries since they do not contain
283+
// the conformance test runner.
284+
fmt.Printf("build %v\n", filepath.Base(protobufPath))
285+
env := os.Environ()
286+
if runtime.GOOS == "darwin" {
287+
// Adding this environment variable appears to be necessary for macOS builds.
288+
env = append(env, "CC=clang")
289+
}
290+
command{
291+
Dir: protobufPath,
292+
Env: env,
293+
}.mustRun(t, "bazel", "build", ":protoc", "//conformance:conformance_test_runner")
258294
}
259-
command{
260-
Dir: protobufPath,
261-
Env: env,
262-
}.mustRun(t, "bazel", "build", ":protoc", "//conformance:conformance_test_runner")
263295
}
264296
check(os.Setenv("PROTOBUF_ROOT", protobufPath)) // for generate-protos
265297
registerBinary("conform-test-runner", filepath.Join(protobufPath, "bazel-bin", "conformance", "conformance_test_runner"))
@@ -298,17 +330,23 @@ func mustInitDeps(t *testing.T) {
298330
check(os.Setenv("GOCACHE", filepath.Join(repoRoot, ".gocache")))
299331
}
300332

301-
func downloadFile(check func(error), dstPath, srcURL string) {
333+
func downloadFile(check func(error), dstPath, srcURL string, perm fs.FileMode) {
302334
resp, err := http.Get(srcURL)
303335
check(err)
304336
defer resp.Body.Close()
337+
if resp.StatusCode != http.StatusOK {
338+
body, _ := io.ReadAll(io.LimitReader(resp.Body, 4<<10))
339+
check(fmt.Errorf("GET %q: non-200 OK status code: %v body: %q", srcURL, resp.Status, body))
340+
}
305341

306342
check(os.MkdirAll(filepath.Dir(dstPath), 0775))
307-
f, err := os.Create(dstPath)
343+
f, err := os.OpenFile(dstPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
308344
check(err)
309345

310346
_, err = io.Copy(f, resp.Body)
311347
check(err)
348+
349+
check(f.Close())
312350
}
313351

314352
func downloadArchive(check func(error), dstPath, srcURL, skipPrefix, wantSHA256 string) {
@@ -317,6 +355,10 @@ func downloadArchive(check func(error), dstPath, srcURL, skipPrefix, wantSHA256
317355
resp, err := http.Get(srcURL)
318356
check(err)
319357
defer resp.Body.Close()
358+
if resp.StatusCode != http.StatusOK {
359+
body, _ := io.ReadAll(io.LimitReader(resp.Body, 4<<10))
360+
check(fmt.Errorf("GET %q: non-200 OK status code: %v body: %q", srcURL, resp.Status, body))
361+
}
320362

321363
var r io.Reader = resp.Body
322364
if wantSHA256 != "" {
@@ -493,3 +535,26 @@ func mustRunCommand(t *testing.T, args ...string) string {
493535
t.Helper()
494536
return command{}.mustRun(t, args...)
495537
}
538+
539+
// race is an approximation of whether the race detector is on.
540+
// It's used to skip the integration test on builders, without
541+
// preventing the integration test from running under the race
542+
// detector as a '//go:build !race' build constraint would.
543+
func race() bool {
544+
bi, ok := debug.ReadBuildInfo()
545+
if !ok {
546+
return false
547+
}
548+
// Use reflect because the debug.BuildInfo.Settings field
549+
// isn't available in Go 1.17.
550+
s := reflect.ValueOf(bi).Elem().FieldByName("Settings")
551+
if !s.IsValid() {
552+
return false
553+
}
554+
for i := 0; i < s.Len(); i++ {
555+
if s.Index(i).FieldByName("Key").String() == "-race" {
556+
return s.Index(i).FieldByName("Value").String() == "true"
557+
}
558+
}
559+
return false
560+
}

regenerate.bash

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,5 @@
33
# Use of this source code is governed by a BSD-style
44
# license that can be found in the LICENSE file.
55

6-
cd "$(git rev-parse --show-toplevel)"
7-
go test -v -timeout=60m -count=1 integration_test.go "$@" -regenerate
6+
go test google.golang.org/protobuf -run='^TestIntegration$' -v -timeout=60m -count=1 "$@" -regenerate
87
exit $?

release.bash

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ fi
7979
git commit -a -m "all: release $(version_string)"
8080

8181
# Build release binaries.
82-
go test -timeout=60m -count=1 integration_test.go "$@" -buildRelease
82+
go test google.golang.org/protobuf -run='^TestIntegration$' -timeout=60m -count=1 "$@" -buildRelease
8383

8484
# Create commit to start development after release.
8585
VERSION_PRERELEASE="${VERSION_PRERELEASE}.devel" # append ".devel"

test.bash

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,5 @@
33
# Use of this source code is governed by a BSD-style
44
# license that can be found in the LICENSE file.
55

6-
cd "$(git rev-parse --show-toplevel)"
7-
go test -v -timeout=60m -count=1 integration_test.go -failfast "$@"
6+
go test google.golang.org/protobuf -run='^TestIntegration$' -v -timeout=60m -count=1 -failfast "$@"
87
exit $?

0 commit comments

Comments
 (0)