From db22d65da41a1bcf9cecb8e4863f2049c2963c8f Mon Sep 17 00:00:00 2001 From: Yevgeniy Miretskiy Date: Thu, 29 Oct 2020 14:28:58 -0400 Subject: [PATCH 1/3] bazel: Add bazel testutils Add a bazel testutils file to help tests use correct paths. Release Notes: None --- pkg/testutils/BUILD.bazel | 1 + pkg/testutils/bazel.go | 120 ++++++++++++++++++++++++++++++++ pkg/testutils/lint/lint_test.go | 1 + 3 files changed, 122 insertions(+) create mode 100644 pkg/testutils/bazel.go diff --git a/pkg/testutils/BUILD.bazel b/pkg/testutils/BUILD.bazel index 3945358adf53..74ce363c0541 100644 --- a/pkg/testutils/BUILD.bazel +++ b/pkg/testutils/BUILD.bazel @@ -4,6 +4,7 @@ go_library( name = "testutils", srcs = [ "base.go", + "bazel.go", "dir.go", "error.go", "files.go", diff --git a/pkg/testutils/bazel.go b/pkg/testutils/bazel.go new file mode 100644 index 000000000000..553a44af891b --- /dev/null +++ b/pkg/testutils/bazel.go @@ -0,0 +1,120 @@ +// Copyright 2020 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. + +package testutils + +import ( + "os" + "path" + "strings" + "testing" + + "github.com/cockroachdb/errors" +) + +// +// Utilities in this file are intended to provide utilities for tests +// to access system resources in a correct way, when executing under bazel. +// +// See https://docs.bazel.build/versions/master/test-encyclopedia.html for more details +// on the directory layout. +// +// When test is compiled with bazel, bazel will create a directory, called RUNFILES directory, +// containing all of the resources required to execute such test (libraries, final binary, +// data resources, etc). +// +// Bazel also sets up various environmental variables that point to the location(s) of +// those resources. +// + +// Name of the environment variable pointing to the absolute path to the base of the RUNFILES tree. +const testSrcDirEnv = "TEST_SRCDIR" + +// Name of the environment variable containing the name of the "workspace". +const testWorkspaceEnv = "TEST_WORKSPACE" + +// Name of the environment variable pointing to the absolute path of the +// temporary directory created for the execution of the test. +const testTmpDirEnv = "TEST_TMPDIR" + +// Name of the environment variable containing the bazel target path (//pkg/cmd/foo:bar). +const testTargetEnv = "TEST_TARGET" + +// RunningUnderBazel returns true if the test is executed by bazel. +func RunningUnderBazel() bool { + return os.Getenv(testSrcDirEnv) != "" +} + +func requireEnv(env string) string { + if v := os.Getenv(env); v != "" { + return v + } + panic(errors.AssertionFailedf("expected value for env: %s", env)) +} + +// TestSrcDir returns the path to the "source" tree. +// +// If running under bazel, this will point to a private, *readonly* +// directory containing symlinks (or copies) of the test data dependencies. +// This directory must be treated readonly. It's an error to try to modify +// anything under this directory: though the operation may succeed, the test +// would not be hermetic, and may fail under other environments. +func TestSrcDir() string { + // If testSrcDirEnv is not set, it means we are not running under bazel, + // and so we can use "" as our directory which should point to the + // src root. + if srcDir := os.Getenv(testSrcDirEnv); srcDir != "" { + return srcDir + } + return "" +} + +// TestTempDir returns a temporary directory and a cleanup function for a test. +func TestTempDir(t testing.TB) (string, func()) { + if RunningUnderBazel() { + // Bazel sets up private temp directories for each test. + return requireEnv(testTmpDirEnv), func() {} + } + return TempDir(t) +} + +// bazeRelativeTargetPath returns relative path to the package +// of the current test. +func bazelRelativeTargetPath() 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, "//") +} + +// TestDataPath returns a path to the directory containing test data files. +// +// Test files are usually checked into the repository under "testdata" directory. +// If we are not using bazel, then the test executes in the directory of +// the actual test, so the files can be referenced via "testdata/subdir/file" relative path. +// +// However, if we are running under bazel, the data files are specified +// via go_test "data" attribute. These files, in turn, are available under RUNFILES directory. +// This helper attempts to construct appropriate path to the RUNFILES directory +// containing test data files, given the relative (to the test) path components. +// +func TestDataPath(relative ...string) string { + if RunningUnderBazel() { + return path.Join(TestSrcDir(), requireEnv(testWorkspaceEnv), bazelRelativeTargetPath(), + path.Join(relative...)) + } + return path.Join(relative...) +} diff --git a/pkg/testutils/lint/lint_test.go b/pkg/testutils/lint/lint_test.go index 8a8eb691ec50..00020afc1e49 100644 --- a/pkg/testutils/lint/lint_test.go +++ b/pkg/testutils/lint/lint_test.go @@ -449,6 +449,7 @@ func TestLint(t *testing.T) { ":!nightly", ":!testutils/lint", ":!util/envutil/env.go", + ":!testutils/bazel.go", ":!util/log/tracebacks.go", ":!util/sdnotify/sdnotify_unix.go", }, From 2c486560c647cb1b3dbaff3df2ca4d06240a0181 Mon Sep 17 00:00:00 2001 From: Yevgeniy Miretskiy Date: Thu, 29 Oct 2020 14:29:50 -0400 Subject: [PATCH 2/3] importccl: Bazelify importccl test Make importccl test hermetic by using correct directories and avoiding test data regeneration. Use 16 shards for the test to speed it up. Release Notes: None --- pkg/ccl/importccl/BUILD.bazel | 3 ++ pkg/ccl/importccl/bench_test.go | 2 +- pkg/ccl/importccl/import_stmt_test.go | 42 +++++++-------------------- 3 files changed, 14 insertions(+), 33 deletions(-) diff --git a/pkg/ccl/importccl/BUILD.bazel b/pkg/ccl/importccl/BUILD.bazel index b4f80bb77271..31de971c2e22 100644 --- a/pkg/ccl/importccl/BUILD.bazel +++ b/pkg/ccl/importccl/BUILD.bazel @@ -107,6 +107,9 @@ go_test( ], data = glob(["testdata/**"]), embed = [":importccl"], + # It's a large test, so use 16 shards to run test cases. + # See https://docs.bazel.build/versions/master/be/common-definitions.html#test.shard_count + shard_count = 16, deps = [ "//pkg/base", "//pkg/blobs", diff --git a/pkg/ccl/importccl/bench_test.go b/pkg/ccl/importccl/bench_test.go index cbd70b41ffb0..e1f521128cbf 100644 --- a/pkg/ccl/importccl/bench_test.go +++ b/pkg/ccl/importccl/bench_test.go @@ -47,7 +47,7 @@ func BenchmarkImportWorkload(b *testing.B) { skip.WithIssue(b, 41932, "broken due to adding keys out-of-order to an sstable") skip.UnderShort(b, "skipping long benchmark") - dir, cleanup := testutils.TempDir(b) + dir, cleanup := testutils.TestTempDir(b) defer cleanup() g := tpcc.FromWarehouses(1) diff --git a/pkg/ccl/importccl/import_stmt_test.go b/pkg/ccl/importccl/import_stmt_test.go index c492250088fa..34a5f001e599 100644 --- a/pkg/ccl/importccl/import_stmt_test.go +++ b/pkg/ccl/importccl/import_stmt_test.go @@ -20,7 +20,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "os" "path/filepath" "regexp" "strings" @@ -1165,7 +1164,7 @@ func TestImportUserDefinedTypes(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) ctx := context.Background() - baseDir, cleanup := testutils.TempDir(t) + baseDir, cleanup := testutils.TestTempDir(t) defer cleanup() tc := testcluster.StartTestCluster( t, 1, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ExternalIODir: baseDir}}) @@ -1264,17 +1263,19 @@ func TestImportUserDefinedTypes(t *testing.T) { }, } - // Set up a directory for the data files. - err := os.Mkdir(filepath.Join(baseDir, "test"), 0777) - require.NoError(t, err) // Test IMPORT INTO. for _, test := range tests { // Write the test data into a file. - err := ioutil.WriteFile(filepath.Join(baseDir, "test", "data"), []byte(test.contents), 0666) + f, err := ioutil.TempFile(baseDir, "data") + require.NoError(t, err) + n, err := f.Write([]byte(test.contents)) require.NoError(t, err) + require.Equal(t, len(test.contents), n) // Run the import statement. sqlDB.Exec(t, fmt.Sprintf("CREATE TABLE t (%s)", test.create)) - sqlDB.Exec(t, fmt.Sprintf("IMPORT INTO t (%s) %s DATA ($1)", test.intoCols, test.typ), "nodelocal://0/test/data") + sqlDB.Exec(t, + fmt.Sprintf("IMPORT INTO t (%s) %s DATA ($1)", test.intoCols, test.typ), + fmt.Sprintf("nodelocal://0/%s", filepath.Base(f.Name()))) // Ensure that the table data is as we expect. sqlDB.CheckQueryResults(t, test.verifyQuery, test.expected) // Clean up after the test. @@ -1379,7 +1380,7 @@ func TestImportCSVStmt(t *testing.T) { blockGC := make(chan struct{}) ctx := context.Background() - baseDir := filepath.Join("testdata", "csv") + baseDir := testutils.TestDataPath("testdata", "csv") tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ SQLMemoryPoolSize: 256 << 20, ExternalIODir: baseDir, @@ -1419,27 +1420,7 @@ func TestImportCSVStmt(t *testing.T) { } // Table schema used in IMPORT TABLE tests. - tablePath := filepath.Join(baseDir, "table") - if err := ioutil.WriteFile(tablePath, []byte(` - CREATE TABLE t ( - a int8 primary key, - b string, - index (b), - index (a, b) - ) - `), 0666); err != nil { - t.Fatal(err) - } schema := []interface{}{"nodelocal://0/table"} - - if err := ioutil.WriteFile(filepath.Join(baseDir, "empty.csv"), nil, 0666); err != nil { - t.Fatal(err) - } - - if err := ioutil.WriteFile(filepath.Join(baseDir, "empty.schema"), nil, 0666); err != nil { - t.Fatal(err) - } - empty := []string{"'nodelocal://0/empty.csv'"} emptySchema := []interface{}{"nodelocal://0/empty.schema"} @@ -2262,7 +2243,7 @@ func TestImportIntoCSV(t *testing.T) { rowsPerRaceFile := 16 ctx := context.Background() - baseDir := filepath.Join("testdata", "csv") + baseDir := testutils.TestDataPath("testdata", "csv") tc := testcluster.StartTestCluster(t, nodes, base.TestClusterArgs{ServerArgs: base.TestServerArgs{ExternalIODir: baseDir}}) defer tc.Stopper().Stop(ctx) conn := tc.Conns[0] @@ -2305,9 +2286,6 @@ func TestImportIntoCSV(t *testing.T) { rowsPerFile = rowsPerRaceFile } - if err := ioutil.WriteFile(filepath.Join(baseDir, "empty.csv"), nil, 0666); err != nil { - t.Fatal(err) - } empty := []string{"'nodelocal://0/empty.csv'"} // Support subtests by keeping track of the number of jobs that are executed. From 2d1bd59afb681dfdc2b5b9c33f7ba926ff5d1f5a Mon Sep 17 00:00:00 2001 From: Yevgeniy Miretskiy Date: Mon, 2 Nov 2020 19:50:54 -0500 Subject: [PATCH 3/3] fixup! bazel: Add bazel testutils --- pkg/testutils/bazel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/testutils/bazel.go b/pkg/testutils/bazel.go index 553a44af891b..7dfdc0b6a100 100644 --- a/pkg/testutils/bazel.go +++ b/pkg/testutils/bazel.go @@ -20,7 +20,7 @@ import ( ) // -// Utilities in this file are intended to provide utilities for tests +// Utilities in this file are intended to provide helpers for tests // to access system resources in a correct way, when executing under bazel. // // See https://docs.bazel.build/versions/master/test-encyclopedia.html for more details