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

Move the e2e tests to a standalone, self dependent go module #1946

Merged
merged 3 commits into from Jun 6, 2023

Conversation

fedepaol
Copy link
Member

@fedepaol fedepaol commented May 19, 2023

Currently the e2e test framework contains a lot of logic, but has some dependencies that root into metallb/internal.
This makes it unpractical (and sometimes impossible) to export the framework to be consumed by other projects (such as the frr daemon being discussed here #1933 ).

On top of that, this means that the metallb's go.mod file is polluted by dependencies that are needed only by tests. Because of this, it makes sense to split the e2e tests in a standalone module that now depends only from the metallb api (and only for the specific metallb tests). This is done by copy-pasting / reimplementing some of the dependencies from the metallb's internal logic, under the spirit that is better to have something self contained (a little copy is better than a little dependency)

Also, fixes #1943

@fedepaol fedepaol force-pushed the standalonetests branch 8 times, most recently from 8525c6d to b62ca49 Compare May 24, 2023 07:46
@@ -48,45 +45,38 @@ require (
github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

there is a comment above "// pinning to specific versions is required because we are consuming
// the k8s.io/kubernetes module from the tests (via the "k8s.io/kubernetes/test/e2e/framework" package)
// See kubernetes/kubernetes#90358 (comment)"

Was this not supposed to delete this require block?

Copy link
Member Author

Choose a reason for hiding this comment

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

We were pinning the deps explicitly only because of the e2e framework dependency. Since now this go.mod does not include the tests, we don't have the e2e framework so we can get rid of this.

On the contrary, we should delete the comment now.

Copy link
Member Author

Choose a reason for hiding this comment

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

removed

go 1.19

use (
./
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need this one?

Copy link
Member Author

Choose a reason for hiding this comment

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

we do, if we remove it we'll get
main module (go.universe.tf/metallb) does not contain package go.universe.tf/metallb/e2etest

So we need the current module (e2e) and the metallb one in order to consume the api not from the most recent tag but from the version we have in the repo.

@mlguerrero12
Copy link
Contributor

LGTM

We make the e2etests a standalone go module, with no dependencies from
the metallb's internal packages.
This is done with a small amount of repetition (the packages that were
consumed directly by metallb/internal are now copied under e2etest/pkg),
but this is for a greater scope which is to make the e2e tests a
standalone module dependant only from the MetalLB API.

This will allow to reuse the infrastructure framework that was created
in the testsuite also for the FRR Daemon, and at some point to move the
e2e framework in a separated repo consumed by both.

Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
This allows to avoid polluting the metallb go.mod with unnecessary
dependencies, and relegates the need for pinning (because of the e2e
framework) only to the e2e go.mod

On top of that, we clean the main go.mod file removing all the
dependencies that were necessary for tests only.

Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
The e2etests are meant to consume the local version of MetalLB. At the
same time, they are (currently) exported as a versioned framework for
implementing tests.

Since they belong to a separate module now, the way they'd work is to
pull the latest tagged version of the MetalLB API, which makes the
development not practical. By adding a local go.work file, we tell the
e2e to consume the local version of the MetalLB module (basically, the
API which is the only remained dependency) so we can evolve MetalLB and
the tests together without incurring in chicken - egg issues.

Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
@fedepaol fedepaol added this pull request to the merge queue Jun 6, 2023
Merged via the queue into metallb:main with commit ef945c2 Jun 6, 2023
24 checks passed
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.

Add a separate go mod for tests
2 participants