Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(tests): require //go:build test tag if importing test packages outside of _test.go files #597

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ run:
# vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
skip-dirs-use-default: true

# Include non-test files tagged as test-only.
# Context: https://github.com/ava-labs/coreth/pull/597
build-tags:
- test

linters:
disable-all: true
enable:
Expand Down
11 changes: 11 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"gopls": {
"build.buildFlags": [
// Context: https://github.com/ava-labs/avalanchego/pull/3173
// Without this tag, the language server won't build the test-only
// code in non-_test.go files.
"--tags='test'",
],
},
"go.testTags": "test",
}
2 changes: 2 additions & 0 deletions core/state/test_statedb.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// (c) 2023, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

//go:build test

package state

import (
Expand Down
2 changes: 2 additions & 0 deletions core/test_blockchain.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// (c) 2020-2021, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

//go:build test

package core

import (
Expand Down
2 changes: 2 additions & 0 deletions precompile/testutils/test_config.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// (c) 2023, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

//go:build test

package testutils

import (
Expand Down
2 changes: 2 additions & 0 deletions precompile/testutils/test_precompile.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// (c) 2023, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

//go:build test

package testutils

import (
Expand Down
2 changes: 2 additions & 0 deletions precompile/testutils/test_predicate.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// (c) 2023, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

//go:build test

package testutils

import (
Expand Down
2 changes: 1 addition & 1 deletion scripts/build_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ source "$CORETH_PATH"/scripts/constants.sh

# We pass in the arguments to this script directly to enable easily passing parameters such as enabling race detection,
# parallelism, and test coverage.
go test -shuffle=on -race -timeout="${TIMEOUT:-600s}" -coverprofile=coverage.out -covermode=atomic ./... "$@"
go test --tags test -shuffle=on -race -timeout="${TIMEOUT:-600s}" -coverprofile=coverage.out -covermode=atomic ./... "$@"
31 changes: 31 additions & 0 deletions scripts/lint_testing_in_non_test_files.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

# Ensure that files not named _test.go files but importing test-related packages
# are constrained with `//go:build test`.

ROOT=$( git rev-parse --show-toplevel )
NON_TEST_GO_FILES=$( find "${ROOT}" -iname '*.go' ! -iname '*_test.go');

IMPORT_TESTING=$( echo "${NON_TEST_GO_FILES}" | xargs grep -lP '^\s*(import\s+)?"testing"');
IMPORT_TESTIFY=$( echo "${NON_TEST_GO_FILES}" | xargs grep -l '"github.com/stretchr/testify');
# TODO(arr4n): send a PR to add support for build tags in `mockgen` and then enable this.
# IMPORT_GOMOCK=$( echo "${NON_TEST_GO_FILES}" | xargs grep -l '"go.uber.org/mock');
HAVE_TEST_LOGIC=$( printf "%s\n%s" "${IMPORT_TESTING}" "${IMPORT_TESTIFY}" );

TAGGED_AS_TEST=$( echo "${NON_TEST_GO_FILES}" | xargs grep -lP '^\/\/go:build\s+(.+(,|\s+))?test[,\s]?');

# -3 suppresses files that have test logic and have the "test" build tag
# -2 suppresses files that are tagged despite not having detectable test logic
UNTAGGED=$( comm -23 <( echo "${HAVE_TEST_LOGIC}" | sort -u ) <( echo "${TAGGED_AS_TEST}" | sort -u ) );
if [ -z "${UNTAGGED}" ];
then
exit 0;
fi

echo "Non-test Go files importing test-only packages MUST have '//go:build test' tag:";
echo "${UNTAGGED}";
exit 1;
2 changes: 2 additions & 0 deletions sync/statesync/test_sync.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// (c) 2021-2022, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

//go:build test

package statesync

import (
Expand Down
2 changes: 2 additions & 0 deletions sync/syncutils/test_trie.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// (c) 2021-2022, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

//go:build test

package syncutils

import (
Expand Down
Loading