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

testutils/buildutil: add disallowed_imports_test bazel macro #79299

Merged

Conversation

ajwerner
Copy link
Contributor

@ajwerner ajwerner commented Apr 4, 2022

This macro can be used to create a test which disallows one go package from
importing another. It uses a thin aspect to accumulate go dependencies while
avoiding any cycle due to nogo.

There's an example of its use in execinfrapb.

Relates heavily to #74176.

Release note: None

Jira issue: CRDB-14797

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@ajwerner ajwerner requested a review from rickystewart April 4, 2022 15:15
Copy link
Collaborator

@rickystewart rickystewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also update generate-test-suites so we can start finding and running these tests in CI.

diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
index fda6ded3a1..8e2b8584c4 100644
--- a/pkg/BUILD.bazel
+++ b/pkg/BUILD.bazel
@@ -266,6 +266,7 @@ ALL_TESTS = [
     "//pkg/sql/doctor:doctor_test",
     "//pkg/sql/enum:enum_test",
     "//pkg/sql/execinfra:execinfra_test",
+    "//pkg/sql/execinfrapb:execinfrapb_disallowed_imports_test",
     "//pkg/sql/execinfrapb:execinfrapb_test",
     "//pkg/sql/execstats:execstats_test",
     "//pkg/sql/flowinfra:flowinfra_test",
diff --git a/pkg/cmd/generate-test-suites/main.go b/pkg/cmd/generate-test-suites/main.go
index 3a2a62b867..0c4481b34d 100644
--- a/pkg/cmd/generate-test-suites/main.go
+++ b/pkg/cmd/generate-test-suites/main.go
@@ -39,7 +39,7 @@ func main() {
 		packagesToQuery = append(packagesToQuery, fmt.Sprintf("//pkg/%s/...", info.Name()))
 	}
 	allPackages := strings.Join(packagesToQuery, "+")
-	queryArgs := []string{"query", fmt.Sprintf("kind(go_test, %s)", allPackages), "--output=label"}
+	queryArgs := []string{"query", fmt.Sprintf("kind(go_test, %s) union kind(sh_test, %s)", allPackages, allPackages), "--output=label"}
 	buf, err := exec.Command("bazel", queryArgs...).Output()
 	if err != nil {
 		log.Printf("Could not query Bazel tests: got error %v", err)

@ajwerner ajwerner force-pushed the ajwerner/bazel-buildutil-imports branch from 7794077 to a0655be Compare April 4, 2022 20:24
@ajwerner ajwerner requested a review from a team as a code owner April 4, 2022 20:24
Copy link
Contributor Author

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

This macro can be used to create a test which disallows one go package from
importing another. It uses a thin aspect to accumulate go dependencies while
avoiding any cycle due to nogo.

There's an example of its use in `execinfrapb`. It also ensures that we
generate the tests into the BUILD.bazel file so they are run by CI.

Relates heavily to cockroachdb#74176.

Release note: None
@ajwerner ajwerner force-pushed the ajwerner/bazel-buildutil-imports branch from a0655be to cd890f5 Compare April 4, 2022 20:25
@ajwerner
Copy link
Contributor Author

ajwerner commented Apr 4, 2022

TFTR!

bors r=rickystewart

@craig
Copy link
Contributor

craig bot commented Apr 4, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Apr 4, 2022

Build succeeded:

@craig craig bot merged commit 11d4026 into cockroachdb:master Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants