Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
馃尡 Add e2e tests for sample external plugin #3419
馃尡 Add e2e tests for sample external plugin #3419
Changes from all commits
597d2d6
926dd60
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See that before init you need to ensure that the plugin was built.
It is not better build the plugin here instead of create the sh?
Could we not move the code to here and to do that in go so that is easier to keep maintained as well?
See that you can run it locally either if you move all to here by just trigging the tests from your IDE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't disagree that it might be easier to maintain, but I think there is a lot of precedence to build stuff in a shell script for the e2e tests. IMO we shouldn't try to reinvent the wheel by adding this to the go code and follow the existing pattern for building and configuring e2e tests with shell scripts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @everettraven,
We currently do not utilize any shell scripting to accomplish the tasks required for our end-to-end (e2e) tests. It's important to note that these tests can be executed locally as long as you have a functional Kubernetes cluster. However, after this PR is merged, this functionality will likely be disrupted and might no longer be possible.
To elaborate, introducing shell scripting for e2e tests as we are doing here will break the development environment. This is contrary to our current setup. The shells scripts in place are Optional to setup the k8s locally either and btw I never used them or to call the test on prow.
Nonetheless, if @Eileen-Yu is able to successfully integrate shell scripting without compromising the functionality (despite my concerns that it will negatively affect the ability to run e2e tests in a development environment), I'm open to having this PR merged and making necessary changes in a follow-up. Please note, however, that it appears these tests are currently failing due to the lack of the necessary steps within the e2e test environment. Therefore, when the init command is called, it results in a failure. See the following for more details:"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @camilamacedo86 , I'm trying to follow your idea:
Since
test/e2e/utils
provides context to bring rich prerequisites, includingk8s cluster
,kubectl
,kubebuilder
.That is how e2e tests get performed for internal plugins.
So, it is not necessary/consistent to deal with prerequisites on a different pattern, such as running a new script.
Is that something you mentioned of and such external-plugin should NOT break?
Here is my opinion:
We have a well-structured directory under
test/e2e
:local.sh
,setup.sh
are used to provide the general environment for e2e testingv4
,deployimage
To maintain that way, when we create a new e2e test suite for external-plugin, it is not necessary/recommended to touch those public files in
1
.Then, if building and configuring external-plugin is needed, we can follow what we did to other plugins: