improve tests #1021
Open
improve tests #1021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
evrardjp
commented
Nov 7, 2024
evrardjp
commented
- Add e2e test concurrency w/ signal
- Add podblocker test
- Rename "version" with "variant" in tests
- Fix Staticcheck's SA1024 (subset with dupe chars)
- Fix Staticcheck's ST1005
- Fix incorrect string prints
- Add staticcheck in make tests
This will help make sure the big refactoring does not break the main features. Signed-off-by: Jean-Philippe Evrard <[email protected]>
Extends test coverage to ensure nothing breaks Signed-off-by: Jean-Philippe Evrard <[email protected]>
For tests not running in different kubernetes versions, but have different tests subcases/variants, rephrase the wording "versions" as it is confusing. Signed-off-by: Jean-Philippe Evrard <[email protected]>
This will replace trim, taking a cutset, with Replace. This clarifies the intent to remove a substring. Signed-off-by: Jean-Philippe Evrard <[email protected]>
According to staticcheck, Error strings should not be capitalized (ST1005). This changes the cases for our errors. Signed-off-by: Jean-Philippe Evrard <[email protected]>
A few strings have evolved to eventually remove all the templating part of their strings, yet kept the formatting features. This is incorrect, and will not pass staticcheck SA1006 and S1039. Signed-off-by: Jean-Philippe Evrard <[email protected]>
Without this, people like myself will forget to run staticcheck. This fixes it by making it part of make tests, which will run with all the fast tests in CI. Signed-off-by: Jean-Philippe Evrard <[email protected]>
|
@dholbach @ckotzbauer can you check this please? |
|
@jackfrancis now that kubecon is over, could you have a looksie at this PR? |
jackfrancis
reviewed
Nov 25, 2024
|
|
||
| randomInt := strconv.Itoa(rand.Intn(100)) | ||
| kindClusterName := fmt.Sprintf("kured-e2e-cordon-%v-%v", variant, randomInt) | ||
| kindClusterConfigFile := "../../.github/kind-cluster-next.yaml" |
There was a problem hiding this comment.
wonder why we're not iterating over all k8s versions for this test?
There was a problem hiding this comment.
Wasted CPU? I am not expecting the behaviour to change across different versions of kubernetes. Hence testing the latest version sounds enough. That reasoning can be applied in most of the tests...
Do you feel we should simplify this?
Here is what I find necessary:
- Test that the reboot works, whether we are using the signal or the command, regardless of the kubernetes version (hence, test all k8s versions).
- Test that the concurrency works regardless of the kubernetes version or the reboot method used
- Test that all advanced features work (blocking with pods, keeping the node cordonned, ...)
WDYT?
jackfrancis
reviewed
Nov 25, 2024
|
|
||
| randomInt := strconv.Itoa(rand.Intn(100)) | ||
| kindClusterName := fmt.Sprintf("kured-e2e-cordon-%v-%v", variant, randomInt) | ||
| kindClusterConfigFile := "../../.github/kind-cluster-next.yaml" |
|
There's a lot here, but as far as I can tell it looks good, added some clarifying questions! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.