Skip to content

Commit

Permalink
Merge #61748
Browse files Browse the repository at this point in the history
61748: bazel: make lint pass unit tests runnable in the sandbox r=rickystewart a=rickystewart

Some unit tests, these lint tests being an example, call into the `go`
binary directly. This is a problem in sandboxed environments where there
may be no globally installed `go` toolchain.

A summary of the changes made here:

1. We depend on the `rules_go` Go library, which surfaces some utilities
   that are useful here as well as generally for porting tests to work
   in Bazel.
2. Add a helper library in `pkg/build/bazel` to surface that stuff.
   There's a stub version of the library as well for code built without
   Bazel; we use build tags to make sure the right version of the
   library is surfaced, and update `.bazelrc` accordingly.
3. Refactor `testutils.TestDataPath` to use the helper lib -- this was
   almost entirely duplicated logic that's outmoded by `rules_go`.
4. Update all the tests to a) depend on the Go SDK when building, and
   b) use the helper library to patch in the toolchain.
5. Finally, delete a bunch of stray `BUILD.bazel` files in the
   `pkg/testutils/lint/passes` directory -- these serve no purpose and
   an old Gazelle configuration put them there. We just forgot to clean
   them up until now.

To test:
  dev test pkg/testutils/lint/passes/... -v

Release justification: Non-production code changes
Release note: None

Co-authored-by: irfan sharif <[email protected]>

Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
craig[bot] and rickystewart committed Mar 10, 2021
2 parents c7f9851 + 80ea53c commit 7a8e7fb
Show file tree
Hide file tree
Showing 49 changed files with 377 additions and 290 deletions.
3 changes: 2 additions & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# TODO(irfansharif): We should fold this into `dev` instead (#56965).

build --ui_event_filters=-DEBUG
build --ui_event_filters=-DEBUG --define gotags=bazel
test --define gotags=bazel
query --ui_event_filters=-DEBUG

try-import %workspace%/.bazelrc.user
1 change: 1 addition & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
#
# gazelle:prefix github.com/cockroachdb/cockroach
# gazelle:build_file_name BUILD.bazel
# gazelle:build_tags bazel

# Enable protobuf generation.
#
Expand Down
7 changes: 7 additions & 0 deletions DEPS.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,13 @@ def go_deps():
sum = "h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUMfuitfgcfuo=",
version = "v0.6.0",
)
go_repository(
name = "com_github_bazelbuild_rules_go",
build_file_proto_mode = "disable_global",
importpath = "github.com/bazelbuild/rules_go",
sum = "h1:2F449QezDZcVW6Jt+kSs8Htd/YI3EXMcvd0aNfVNCI4=",
version = "v0.26.0",
)

go_repository(
name = "com_github_benesch_cgosymbolizer",
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ require (
github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e
github.com/aws/aws-sdk-go v1.36.33
github.com/axiomhq/hyperloglog v0.0.0-20181223111420-4b99d0c2c99e
github.com/bazelbuild/rules_go v0.26.0
github.com/benesch/cgosymbolizer v0.0.0-20180702220239-70e1ee2b39d3
github.com/biogo/store v0.0.0-20160505134755-913427a1d5e8
github.com/cenkalti/backoff v2.1.1+incompatible
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ github.com/aws/aws-sdk-go v1.36.33/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2z
github.com/axiomhq/hyperloglog v0.0.0-20181223111420-4b99d0c2c99e h1:190ugM9MsyFauTkR/UqcHG/mn5nmFe6SvHJqEHIrtrA=
github.com/axiomhq/hyperloglog v0.0.0-20181223111420-4b99d0c2c99e/go.mod h1:IOXAcuKIFq/mDyuQ4wyJuJ79XLMsmLM+5RdQ+vWrL7o=
github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g=
github.com/bazelbuild/rules_go v0.26.0 h1:2F449QezDZcVW6Jt+kSs8Htd/YI3EXMcvd0aNfVNCI4=
github.com/bazelbuild/rules_go v0.26.0/go.mod h1:MC23Dc/wkXEyk3Wpq6lCqz0ZAYOZDw2DR5y3N1q2i7M=
github.com/benesch/cgosymbolizer v0.0.0-20180702220239-70e1ee2b39d3 h1:Llg88pHOiUbcFOFhr009G8fOBQL6gaVDnxUUeWZuLog=
github.com/benesch/cgosymbolizer v0.0.0-20180702220239-70e1ee2b39d3/go.mod h1:eMD2XUcPsHYbakFEocKrWZp47G0MRJYoC60qFblGjpA=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
Expand Down
4 changes: 2 additions & 2 deletions pkg/bench/ddl_analysis/validate_benchmark_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func resultsToExpectations(results []benchmarkResult) benchmarkExpectations {
}

func writeExpectationsFile(t *testing.T, expectations benchmarkExpectations) {
f, err := os.Create(testutils.TestDataPath("testdata", expectationsFilename))
f, err := os.Create(testutils.TestDataPath(t, expectationsFilename))
require.NoError(t, err)
defer func() { require.NoError(t, f.Close()) }()
w := csv.NewWriter(f)
Expand All @@ -225,7 +225,7 @@ func writeExpectationsFile(t *testing.T, expectations benchmarkExpectations) {
}

func readExpectationsFile(t *testing.T) benchmarkExpectations {
f, err := os.Open(testutils.TestDataPath("testdata", expectationsFilename))
f, err := os.Open(testutils.TestDataPath(t, expectationsFilename))
require.NoError(t, err)
defer func() { _ = f.Close() }()

Expand Down
9 changes: 9 additions & 0 deletions pkg/build/bazel/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "bazel",
srcs = ["bazel.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/build/bazel",
visibility = ["//visibility:public"],
deps = ["@io_bazel_rules_go//go/tools/bazel:go_default_library"],
)
91 changes: 91 additions & 0 deletions pkg/build/bazel/bazel.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright 2015 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

// +build bazel

package bazel

import (
"fmt"
"os"
"path"
"path/filepath"
"strings"

inner "github.com/bazelbuild/rules_go/go/tools/bazel"
)

// Return true iff this library was built with Bazel.
func BuiltWithBazel() bool {
return true
}

// FindBinary is a convenience wrapper around the rules_go variant.
func FindBinary(pkg, name string) (string, bool) {
return inner.FindBinary(pkg, name)
}

// Runfile is a convenience wrapper around the rules_go variant.
func Runfile(path string) (string, error) {
return inner.Runfile(path)
}

// RunfilePath is a convenience wrapper around the rules_go variant.
func RunfilesPath() (string, error) {
return inner.RunfilesPath()
}

// TestTmpDir is a convenience wrapper around the rules_go variant.
func TestTmpDir() string {
return inner.TestTmpDir()
}

// Updates the current environment to use the Go toolchain that Bazel built this
// binary/test with (updates the `PATH`/`GOROOT`/`GOCACHE` environment
// variables).
// If you want to use this function, your binary/test target MUST have
// `@go_sdk//:files` in its `data` -- this will make sure the whole toolchain
// gets pulled into the sandbox as well. Generally, this function should only
// be called in init().
func SetGoEnv() {
gobin, err := Runfile("bin/go")
if err != nil {
panic(err)
}

if err := os.Setenv("PATH", fmt.Sprintf("%s%c%s", filepath.Dir(gobin), os.PathListSeparator, os.Getenv("PATH"))); err != nil {
panic(err)
}
if err := os.Setenv("GOROOT", filepath.Dir(filepath.Dir(gobin))); err != nil {
panic(err)
}
if err := os.Setenv("GOCACHE", path.Join(inner.TestTmpDir(), ".gocache")); err != nil {
panic(err)
}
}

// Name of the environment variable containing the bazel target path
// (//pkg/cmd/foo:bar).
const testTargetEnv = "TEST_TARGET"

// RelativeTestTargetPath returns relative path to the package
// of the current test.
func RelativeTestTargetPath() string {
target := os.Getenv(testTargetEnv)
if target == "" {
return ""
}

// Drop target name.
if last := strings.LastIndex(target, ":"); last > 0 {
target = target[:last]
}
return strings.TrimPrefix(target, "//")
}
46 changes: 46 additions & 0 deletions pkg/build/bazel/non_bazel.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2015 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

// +build !bazel

package bazel

// This file contains stub implementations for non-bazel builds.
// See bazel.go for full documentation on the contracts of these functions.

// BuiltWithBazel returns true iff this library was built with Bazel.
func BuiltWithBazel() bool {
return false
}

// Runfile is not implemented.
func Runfile(string) (string, error) {
panic("not built with Bazel")
}

// RunfilesPath is not implemented.
func RunfilesPath() (string, error) {
panic("not built with Bazel")
}

// TestTmpDir is not implemented.
func TestTmpDir() string {
panic("not built with Bazel")
}

// RelativeTestTargetPath is not implemented.
func RelativeTestTargetPath() string {
panic("not built with Bazel")
}

// SetGoEnv is not implemented.
func SetGoEnv() {
panic("not built with Bazel")
}
4 changes: 2 additions & 2 deletions pkg/ccl/importccl/import_stmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1712,7 +1712,7 @@ func TestImportCSVStmt(t *testing.T) {
blockGC := make(chan struct{})

ctx := context.Background()
baseDir := testutils.TestDataPath("testdata", "csv")
baseDir := testutils.TestDataPath(t, "csv")
tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: base.TestServerArgs{
SQLMemoryPoolSize: 256 << 20,
ExternalIODir: baseDir,
Expand Down Expand Up @@ -2611,7 +2611,7 @@ func TestImportIntoCSV(t *testing.T) {
rowsPerRaceFile := 16

ctx := context.Background()
baseDir := testutils.TestDataPath("testdata", "csv")
baseDir := testutils.TestDataPath(t, "csv")
tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ExternalIODir: baseDir}})
defer tc.Stopper().Stop(ctx)
conn := tc.Conns[0]
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/catalog/catalogkv/unwrap_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
// will hold a file "descriptors.csv" which is a csv of id,descriptor where
// descriptor is hex encoded.
func TestUnwrapValidation(t *testing.T) {
testdata := testutils.TestDataPath("testdata", "unwrap_validation")
testdata := testutils.TestDataPath(t, "unwrap_validation")
const descriptorsCSVFilename = "descriptors.csv"
dirs, err := ioutil.ReadDir(testdata)
require.NoError(t, err)
Expand Down
4 changes: 3 additions & 1 deletion pkg/testutils/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go_library(
name = "testutils",
srcs = [
"base.go",
"bazel.go",
"data_path.go",
"dir.go",
"error.go",
"files.go",
Expand All @@ -20,6 +20,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/base",
"//pkg/build/bazel",
"//pkg/roachpb",
"//pkg/security",
"//pkg/sql/pgwire/pgerror",
Expand All @@ -30,6 +31,7 @@ go_library(
"//pkg/util/timeutil",
"//pkg/util/tracing",
"@com_github_cockroachdb_errors//:errors",
"@com_github_stretchr_testify//require",
],
)

Expand Down
110 changes: 0 additions & 110 deletions pkg/testutils/bazel.go

This file was deleted.

Loading

0 comments on commit 7a8e7fb

Please sign in to comment.