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

[Breaking] Fix issue where we have duplicate packages in tests #301

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Tatskaari
Copy link
Contributor

@Tatskaari Tatskaari commented Sep 24, 2024

When we build tests that aren't external, we have two versions for the package under test:

  1. the package from the go_library() target that we're trying to test
  2. the _{name}#lib package that re-compiles that package with all the test sources

Before we would have two different import paths for these two versions of the package. They're both available in the test binary, where the test version of the package is available as import "{name}_test_lib".

This causes a bunch of problems and confusion:

  1. The package initialisation might happen twice which has causes issues registering various things that are meant to be unique
  2. Components that are compiled against the production package expect the production types, which are not compatible with the test version

There are a few cases internally where a non-exported test imports the non-test version of the package to avoid type conflicts between the two versions of the packages.

This PR links the test version of the package into the binary, with the same import path as the prod version. This means there's no distinction.

TODO gate this behind a feature flag.

@Tatskaari Tatskaari force-pushed the fix-duplicate-package-in-test branch from d2d0cb1 to c5af7be Compare September 28, 2024 15:51
@Tatskaari Tatskaari changed the title Fix issue where we have duplicate packages in tests [Breaking] Fix issue where we have duplicate packages in tests Oct 1, 2024
@peterebden
Copy link
Contributor

Why is this breaking? Is it just because it stops this behaviour of registering things twice?

@Tatskaari
Copy link
Contributor Author

Tatskaari commented Oct 1, 2024

Why is this breaking? Is it just because it stops this behaviour of registering things twice?

Yeah, there are a number of places in core3 where a test package imports itself. This is fine right now because the test vs. prod packages are different. Going forward this will be a cyclic import.

@peterebden
Copy link
Contributor

Why is this breaking? Is it just because it stops this behaviour of registering things twice?

Yeah, there are a number of places in core3 where a test package imports itself. This is fine right now because the test vs. prod packages are different. Going forward this will be a cyclic import.

Right, thanks.

I think those are degenerate and they shouldn't have been doing that (albeit it was allowed and so eventually someone will try to do that thing). I don't think that would have worked with go build.

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