-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
feat: refactor minikube test bot message #19036
Conversation
/ok-to-test |
This comment has been minimized.
This comment has been minimized.
@ComradeProgrammer lets make it in a way that if it has more than 10 failures in an environment it should be skipped and instead just a english message that this enviroment had ALOT of failures... |
08f2d4a
to
ada8e6b
Compare
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
67fcdb5
to
7a50327
Compare
Co-authored-by: Steven Powell <[email protected]>
7a50327
to
fecb7f3
Compare
This comment has been minimized.
This comment has been minimized.
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.
Looking at your example output:
Environment | Test Name | Flake Rate |
---|---|---|
Docker_Linux_crio (2 failed) | TestAddons/parallel/Ingress(gopogh) | 100.00% (chart) |
Docker_Linux_crio (2 failed) | TestAddons/parallel/MetricsServer(gopogh) | 100.00% (chart) |
Docker_macOS (1 failed) | TestSkaffold(gopogh) | 93.10% (chart) |
I think we should have a flake threshold that we don't show. Showing flakes that are at or near 100% isn't very helpful, it's just noise that we have to read through to find failures that we care about. In the existing flake rate comment we only show tests that have less than 50% flake rate if ($3 < 50) printf "%s:%s,%s\n", $1, $2, $3
, I think we should implement something similar.
thats a a good idea |
50c8c07
to
8a6a254
Compare
8a6a254
to
0c853be
Compare
Now tests with >50% flake rate will not be shown |
This comment has been minimized.
This comment has been minimized.
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.
thank you
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ComradeProgrammer, medyagh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
result := map[string]map[string]float64{} | ||
for i := 1; i < len(records); i++ { | ||
// for each line in csv we extract env, test name and flake rate | ||
if len(records[i]) < 2 { |
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.
if len(records[i]) < 2 { | |
if len(records[i]) < 3 { |
records[i][2]
is below, so need to increase length check
@ComradeProgrammer can you plz take a look |
9f3b817
to
8775df0
Compare
Co-authored-by: Steven Powell <[email protected]>
8775df0
to
97b8ec3
Compare
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.
Good work, looks really good
// $3 is the file containing a list of finished environments, one item per line | ||
func main() { | ||
ctx := context.Background() | ||
client, err := storage.NewClient(context.Background()) |
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.
client, err := storage.NewClient(context.Background()) | |
client, err := storage.NewClient(ctx) |
btk := client.Bucket("minikube-builds") | ||
obj := btk.Object(fmt.Sprintf("logs/%s/%s/%s_summary.json", pr, rootJob, env)) |
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.
btk := client.Bucket("minikube-builds") | |
obj := btk.Object(fmt.Sprintf("logs/%s/%s/%s_summary.json", pr, rootJob, env)) | |
bkt := client.Bucket("minikube-builds") | |
obj := bkt.Object(fmt.Sprintf("logs/%s/%s/%s_summary.json", pr, rootJob, env)) |
btk := client.Bucket("minikube-flake-rate") | ||
obj := btk.Object("flake_rates.csv") |
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.
btk := client.Bucket("minikube-flake-rate") | |
obj := btk.Object("flake_rates.csv") | |
bkt := client.Bucket("minikube-flake-rate") | |
obj := bkt.Object("flake_rates.csv") |
for i := 1; i < len(records); i++ { | ||
// for each line in csv we extract env, test name and flake rate | ||
if len(records[i]) < 3 { | ||
// the csv must have at least 2 columns |
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.
// the csv must have at least 2 columns | |
// the csv must have at least 3 columns |
|
||
} | ||
|
||
// flakeRate downloads recent flake rates from GCS, and returns a map{env->map{testname->flake rate}} |
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.
// flakeRate downloads recent flake rates from GCS, and returns a map{env->map{testname->flake rate}} | |
// flakeRate downloads recent flake rates from GCS, and returns a | |
// map{env->map{testname->flake rate}} |
Shortening long comments for better readability
kvm2 driver with docker runtime
Times for minikube ingress: 23.9s 24.4s 27.9s 24.9s 23.9s Times for minikube start: 47.7s 51.1s 51.8s 50.2s 49.8s docker driver with docker runtime
Times for minikube start: 21.3s 21.3s 21.0s 21.6s 21.5s Times for minikube (PR 19036) ingress: 21.7s 22.8s 21.8s 21.8s 21.8s docker driver with containerd runtime
Times for minikube start: 23.5s 19.6s 23.0s 19.8s 19.9s Times for minikube (PR 19036) ingress: 32.2s 31.8s 32.2s 32.2s 32.2s |
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
Co-authored-by: Steven Powell <[email protected]>
After:
Here are the number of top 10 failed tests in each environments with lowest flake rate.
Besides the following environments also have failed tests:
Docker_Windows: 3 failed (gopogh)
Docker_Linux_containerd_arm64: 6 failed (gopogh)
Hyper-V_Windows: 14 failed (gopogh)
Docker_Linux_docker_arm64: 1 failed (gopogh)
Docker_Linux_crio: 2 failed (gopogh)
QEMU_macOS: 156 failed (gopogh)
Docker_macOS: 1 failed (gopogh)
KVM_Linux_crio: 31 failed (gopogh)
To see the flake rates of all tests by environment, click here.