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

Proposal: Unify tests to _test packages and remove dot imports #45

Closed
wants to merge 1 commit into from

Conversation

jchadwick-buf
Copy link
Member

The export_test.go pattern can be used to make all of our tests use the _test pattern and ensure that the internal API surface we depend on in tests is explicitly defined. While not strictly necessary, this PR unifies tests using this strategy. Also, I ran go mod tidy and removed the one instance of dot imports (otherwise unused.)

The only package where this is somewhat questionable IMO is httplb where the test-exported surface is a bit larger than ideal, but even so, I still think it's not a bad idea to keep it accounted for.

@jchadwick-buf jchadwick-buf requested a review from jhump June 30, 2023 19:52
Copy link
Member

@jhump jhump left a comment

Choose a reason for hiding this comment

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

I gotta be honest: I don't understand the motivation here. If tests need to use internal code, it's best to just leave the tests in the same package as code under test. I think we only need to move that sort of code to a _test package to break an import cycle.

@jchadwick-buf jchadwick-buf deleted the jchadwick/unify-tests branch July 6, 2023 14:18
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.

2 participants