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

Test code more accurately detects dependencies #153

Merged
merged 7 commits into from
Mar 2, 2023

Conversation

illicitonion
Copy link
Collaborator

In particular, two classes of dependency are now better handled:

  1. Test code for which there isn't a corresponding non-test package no longer emits a warning about a missing package.
  2. Test code can now depend on test utility helpers which are defined in other test packages, in very specific circumstances.

I strongly recommend reviewing commit-by-commit.

Fixes #136

Copy link
Collaborator

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

Seems fine, but I have questions. They're not serious enough to block landing the PR, though.

java/gazelle/resolve_test.go Outdated Show resolved Hide resolved
java/gazelle/private/types/types.go Outdated Show resolved Hide resolved
java/gazelle/private/types/types.go Show resolved Hide resolved
Before, some slices of strings contained both packages and strings, and
we weren't clearly handling each correctly.

Instead, be explicit about where we're using each.
Previously we were treating testonly libraries as unique packages in
their own namespace. Instead, treat them as having the correct package
name, but embed the "is testonly" data in a struct so we can choose to
filter on it flexibly as we wish.
Previously, test targets assumed that they had matching production code
they were testing, and would error if it couldn't be found.

Instead, treat this as a normal occurrence, as often test code exists in
its own package with no directly corresponding production code package.
This only works if there was no non-test package providing the same
package.

We don't automatically set the visibility for the depended-on code -
this is an explicit decision that should be made by a human as to
whether that dependency should be allowed.
@illicitonion illicitonion merged commit 42c3f59 into bazel-contrib:main Mar 2, 2023
@illicitonion illicitonion deleted the foreign-testonly branch March 2, 2023 13:40
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.

[Gazelle] Resolve deps that are testonly libraries in the test hierarchy
2 participants