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

Consider releasing the our test folder as a separate go module #4687

Closed
vincepri opened this issue May 27, 2021 · 4 comments · Fixed by #4713
Closed

Consider releasing the our test folder as a separate go module #4687

vincepri opened this issue May 27, 2021 · 4 comments · Fixed by #4713
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@vincepri
Copy link
Member

There is a couple of PRs that introduce a dependency on docker (#4499 and #4413) which would become a dependency in the main Cluster API go.mod.

While these are good improvements, it'd be great to completely separate everything under testing into a different go module and release the module separately from the main module.

These are the changes that would need to happen:

  • Run go mod init under test/ folder.
  • Remove the go module from test/infrastructure/docker.
  • Move the test/helpers into util/test folder (this is primarily envtest, shouldn't need to be pushed separately).
  • Update the release guide to make sure every time a new tag is cut, one is also cut for test. For example, if we're releasing v0.4.20, we should also tag test/v0.4.20.
  • Update the v1alpha3-to-v1alpha4 provider implementers guide to make sure that folks import the testing framework from the new module.

/kind cleanup
/kind api-change
/priority important-soon

cc @stmcginnis @CecileRobertMichon @elmiko @fabriziopandini @yastij @sedefsavas

If this change is accepted, we should mark it as a potential release blocker.

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels May 27, 2021
@fabriziopandini
Copy link
Member

I'm +100 to move envtest env into util/test (and finally drop the test/helpers folder)

I'm also ok with the go mod reorg; this has the nice side effect that kind will be removed from top level CAPI dependencies

WRT to introduce docker as a dependency, this is an interesting change, and hopefully it will help us in having better error messages in case of problems (vs shelling out). However, In my mind the primary goal of this effort still remains to make CAPD unit testable, so we can eventually reshuffle priorities to introduce the interface required for decoupling docker call first, and change how do we make docker calls only after this. This would allow us some more time before taking this decision.

@yastij
Copy link
Member

yastij commented May 28, 2021

+1 from me

@elmiko
Copy link
Contributor

elmiko commented Jun 1, 2021

+1, preventing some of those top level dependencies seems really worthwhile.

@stmcginnis
Copy link
Contributor

Would it make sense to separate the docker engine interaction from the rest of the test code? And keep CAPD separate too?

The changes being proposed for not exec'ing docker calls impacts test infra and CAPD. It might be nice to keep these independent. Then if someone wants to use CAPD outside of our testing, things can be limited to just the parts needed. Vice versa, for a user of the test infrastructure running against a different provider, the unused CAPD bits can be kept separate.

If we would want to keep CAPD separate, then it would make sense to move the new docker interfacing code to its own module so it can be picked up as a dependency separately by test infra and CAPD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants