-
Notifications
You must be signed in to change notification settings - Fork 205
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
improve tests #1021
base: main
Are you sure you want to change the base?
improve tests #1021
Conversation
evrardjp
commented
Nov 7, 2024
- 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? |
|
||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder why we're not iterating over all k8s versions for this test?
|
||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto above comment
There's a lot here, but as far as I can tell it looks good, added some clarifying questions! |