Skip to content

Commit

Permalink
lint: make runnable under bazel test
Browse files Browse the repository at this point in the history
This test was never directly runnable under `bazel test`, and instead
required using `bazel run` with the wrapper target
`//build/bazelutil:lint` that set up some dependencies and some
environment variables appropriately. Instead, we simply pass in those
environment variables from the CI script or from `dev`, and add extra
dependencies to the `lint_test` target (`crlfmt`, `optfmt`) so the test
can use them.

It's necessary to pass `GO_SDK` as a path to the Bazel-provided Go SDK
and `COCKROACH_WORKSPACE` as the path to the checkout as this stuff will
not be found in the sandbox.

Epic: CRDB-8308
Release note: None
  • Loading branch information
rickystewart committed Mar 6, 2024
1 parent c497caa commit dc26091
Show file tree
Hide file tree
Showing 15 changed files with 189 additions and 238 deletions.
6 changes: 0 additions & 6 deletions build/bazelutil/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ exports_files(["nogo_config.json"])

load("@bazel_skylib//rules:analysis_test.bzl", "analysis_test")
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load(":lint.bzl", "lint_binary")

analysis_test(
name = "test_nogo_configured",
Expand All @@ -25,11 +24,6 @@ genrule(
}),
)

lint_binary(
name = "lint",
test = "//pkg/testutils/lint:lint_test",
)

# This noop target is a workaround for https://github.com/bazelbuild/bazel-gazelle/issues/1078.
# We use it in //pkg/kv/kvclient/{kvcoord,rangefeed}.
go_library(
Expand Down
56 changes: 0 additions & 56 deletions build/bazelutil/lint.bzl

This file was deleted.

46 changes: 0 additions & 46 deletions build/bazelutil/lint.sh.in

This file was deleted.

19 changes: 13 additions & 6 deletions build/teamcity/cockroach/ci/tests/lint_impl.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,22 @@

set -xeuo pipefail

dir="$(dirname $(dirname $(dirname $(dirname $(dirname "${0}")))))"

source "$dir/teamcity-support.sh" # For $root

# GCAssert and unused need generated files in the workspace to work properly.
# generated files requirements -- begin
bazel run //pkg/gen:code
bazel run //pkg/cmd/generate-cgo:generate-cgo --run_under="cd $(bazel info workspace) && "
bazel run //pkg/cmd/generate-cgo:generate-cgo --run_under="cd $root && "
# generated files requirements -- end

bazel build //pkg/cmd/bazci --config=ci
XML_OUTPUT_FILE=/artifacts/test.xml GO_TEST_WRAP_TESTV=1 GO_TEST_WRAP=1 CC=$(which gcc) CXX=$(which gcc) bazel \
run --config=ci --config=test //build/bazelutil:lint
# The schema of the output test.xml will be slightly wrong -- ask `bazci` to fix
# it up.
$(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci munge-test-xml /artifacts/test.xml
$(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci -- test \
//pkg/testutils/lint:lint_test --config=ci \
--test_env=CC=$(which gcc) \
--test_env=CXX=$(which gcc) \
--test_env=HOME \
--sandbox_writable_path=$HOME \
--test_env=GO_SDK=$(dirname $(dirname $(bazel run @go_sdk//:bin/go --run_under=realpath))) \
--test_env=COCKROACH_WORKSPACE=$root
21 changes: 11 additions & 10 deletions build/teamcity/cockroach/nightlies/lint_urls_impl.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,20 @@ set -xeuo pipefail

dir="$(dirname $(dirname $(dirname $(dirname "${0}"))))"

source "$dir/teamcity-support.sh" # For $root

# GCAssert and unused need generated files in the workspace to work properly.
# generated files requirements -- begin
bazel run //pkg/gen:code
bazel run //pkg/cmd/generate-cgo:generate-cgo --run_under="cd $(bazel info workspace) && "
bazel run //pkg/cmd/generate-cgo:generate-cgo --run_under="cd $root && "
# generated files requirements -- end

bazel build //pkg/cmd/bazci --config=ci
BAZEL_BIN=$(bazel info bazel-bin --config=ci)
exit_status=0
XML_OUTPUT_FILE=/artifacts/test.xml GO_TEST_WRAP_TESTV=1 GO_TEST_WRAP=1 bazel \
run --config=ci --config=test --define gotags=bazel,gss,nightly,lint \
//build/bazelutil:lint || exit_status=$?
# The schema of the output test.xml will be slightly wrong -- ask `bazci` to fix
# it up.
$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci munge-test-xml /artifacts/test.xml
exit $exit_status
$(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci -- \
test --config=ci --define gotags=bazel,gss,nightly,lint \
--test_env=CC=$(which gcc) \
--test_env=CXX=$(which gcc) \
--test_env=HOME \
--sandbox_writable_path=$HOME \
--test_env=GO_SDK=$(dirname $(dirname $(bazel run @go_sdk//:bin/go --run_under=realpath))) \
--test_env=COCKROACH_WORKSPACE=$root
2 changes: 1 addition & 1 deletion dev
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fi
set -euo pipefail

# Bump this counter to force rebuilding `dev` on all machines.
DEV_VERSION=91
DEV_VERSION=92

THIS_DIR=$(cd "$(dirname "$0")" && pwd)
BINARY_DIR=$THIS_DIR/bin/dev-versions
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/datadriven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestDataDriven(t *testing.T) {
osOpts := []os.Option{
os.WithLogger(log.New(logger, "", 0)),
os.WithDryrun(),
os.WithIntercept("echo $HOME", testFixturesDirPlaceholder),
os.WithIntercept("echo $HOME/.cache", testFixturesDirPlaceholder),
}

if !verbose { // suppress all internal output unless told otherwise
Expand Down
16 changes: 15 additions & 1 deletion pkg/cmd/dev/io/os/os.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ func (o *OS) CurrentUserAndGroup() (uid string, gid string, err error) {

// UserCacheDir returns the cache directory for the current user if possible.
func (o *OS) UserCacheDir() (dir string, err error) {
command := "echo $HOME"
command := "echo $HOME/.cache"
if !o.knobs.silent {
o.logger.Print(command)
}
Expand All @@ -548,6 +548,20 @@ func (o *OS) UserCacheDir() (dir string, err error) {

}

// UserCacheDir returns the cache directory for the current user if possible.
func (o *OS) HomeDir() (dir string, err error) {
command := "echo $HOME"
if !o.knobs.silent {
o.logger.Print(command)
}

dir, err = o.Next(command, func() (dir string, err error) {
return os.UserHomeDir()
})

return strings.TrimSpace(dir), err
}

// Next is a thin interceptor for all os activity, running them through
// testing knobs first.
func (o *OS) Next(command string, f func() (output string, err error)) (string, error) {
Expand Down
71 changes: 42 additions & 29 deletions pkg/cmd/dev/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ package main
import (
"fmt"
"log"
"os"
"path/filepath"
"strings"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -53,8 +53,42 @@ func (d *dev) lint(cmd *cobra.Command, commandLine []string) error {
}
}

lintEnv := os.Environ()

var args []string
args = append(args, "test", "//pkg/testutils/lint:lint_test")
if numCPUs != 0 {
args = append(args, fmt.Sprintf("--local_cpu_resources=%d", numCPUs))
}
args = append(args, additionalBazelArgs...)
args = append(args, "--test_arg", "-test.v")
if short {
args = append(args, "--test_arg", "-test.short")
}
if timeout > 0 {
args = append(args, fmt.Sprintf("--test_timeout=%d", int(timeout.Seconds())))
}
if filter != "" {
args = append(args, fmt.Sprintf("--test_filter=Lint/%s", filter))
}
workspace, err := d.getWorkspace(ctx)
if err != nil {
return err
}
args = append(args, fmt.Sprintf("--test_env=COCKROACH_WORKSPACE=%s", workspace))
home, err := d.os.HomeDir()
if err != nil {
return err
}
args = append(args,
fmt.Sprintf("--test_env=HOME=%s", home),
fmt.Sprintf("--sandbox_writable_path=%s", home))
args = append(args, "--test_output", "streamed")
// We need the location of the Go SDK for staticcheck and gcassert.
out, err := d.exec.CommandContextSilent(ctx, "bazel", "run", "@go_sdk//:bin/go", "--run_under=//build/bazelutil/whereis")
if err != nil {
return fmt.Errorf("could not find location of Go SDK (%w)", err)
}
goBin := strings.TrimSpace(string(out))
args = append(args, fmt.Sprintf("--test_env=GO_SDK=%s", filepath.Dir(filepath.Dir(goBin))))
if !short {
// First, generate code to make sure GCAssert and any other
// tests that depend on generated code still work.
Expand All @@ -67,30 +101,11 @@ func (d *dev) lint(cmd *cobra.Command, commandLine []string) error {
return fmt.Errorf("`cc` is not installed; needed for `TestGCAssert` (%w)", err)
}
cc = strings.TrimSpace(cc)
d.log.Printf("export CC=%s", cc)
d.log.Printf("export CXX=%s", cc)
envWithCc := []string{"CC=" + cc, "CXX=" + cc}
lintEnv = append(envWithCc, lintEnv...)
args = append(args,
fmt.Sprintf("--test_env=CC=%s", cc),
fmt.Sprintf("--test_env=CXX=%s", cc))
}

var args []string
// NOTE the --config=test here. It's very important we compile the test binary with the
// appropriate stuff (gotags, etc.)
args = append(args, "run", "--config=test", "//build/bazelutil:lint")
if numCPUs != 0 {
args = append(args, fmt.Sprintf("--local_cpu_resources=%d", numCPUs))
}
args = append(args, additionalBazelArgs...)
args = append(args, "--", "-test.v")
if short {
args = append(args, "-test.short")
}
if timeout > 0 {
args = append(args, "-test.timeout", timeout.String())
}
if filter != "" {
args = append(args, "-test.run", fmt.Sprintf("Lint/%s", filter))
}
logCommand("bazel", args...)
if len(pkgs) > 1 {
return fmt.Errorf("can only lint a single package (found %s)", strings.Join(pkgs, ", "))
Expand All @@ -100,11 +115,9 @@ func (d *dev) lint(cmd *cobra.Command, commandLine []string) error {
if !strings.HasPrefix(pkg, "./") {
pkg = "./" + pkg
}
envvar := fmt.Sprintf("PKG=%s", pkg)
d.log.Printf("export %s", envvar)
lintEnv = append(lintEnv, envvar)
args = append(args, fmt.Sprintf("--test_env=PKG=%s", pkg))
}
err := d.exec.CommandContextWithEnv(ctx, lintEnv, "bazel", args...)
err = d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...)
if err != nil {
return err
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/cmd/dev/testdata/datadriven/bench
Original file line number Diff line number Diff line change
@@ -1,36 +1,36 @@
exec
dev bench pkg/spanconfig/...
----
echo $HOME
echo $HOME/.cache
bazel test pkg/spanconfig/...:all --test_arg -test.run=- --test_arg -test.bench=. --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors

exec
dev bench pkg/sql/parser --filter=BenchmarkParse
----
echo $HOME
echo $HOME/.cache
bazel test pkg/sql/parser:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors

exec
dev bench pkg/sql/parser --filter=BenchmarkParse --stream-output
----
echo $HOME
echo $HOME/.cache
bazel test pkg/sql/parser:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed

exec
dev bench pkg/bench -f=BenchmarkTracing/1node/scan/trace=off --count=2 --bench-time=10x --bench-mem
----
echo $HOME
echo $HOME/.cache
bazel test pkg/bench:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --test_arg -test.count=2 --test_arg -test.benchtime=10x --test_arg -test.benchmem --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors

exec
dev bench pkg/spanconfig/spanconfigkvsubscriber -f=BenchmarkSpanConfigDecoder --cpus=10 --ignore-cache -v --timeout=50s
----
echo $HOME
echo $HOME/.cache
bazel test --local_cpu_resources=10 --test_timeout=50 pkg/spanconfig/spanconfigkvsubscriber:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkSpanConfigDecoder --test_sharding_strategy=disabled --test_arg -test.v --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output all

exec
dev bench pkg/bench -f='BenchmarkTracing/1node/scan/trace=off' --test-args '-test.memprofile=mem.out -test.cpuprofile=cpu.out'
----
bazel info workspace --color=no
echo $HOME
echo $HOME/.cache
bazel test pkg/bench:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --crdb_test_off --test_arg -test.outputdir=crdb-checkout --sandbox_writable_path=crdb-checkout --test_arg -test.memprofile=mem.out --test_arg -test.cpuprofile=cpu.out --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors
Loading

0 comments on commit dc26091

Please sign in to comment.