-
Notifications
You must be signed in to change notification settings - Fork 169
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
mantle/kola/harness.go: add snooze support for tests #2307
Conversation
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.
Looks sane to me! Just one minor comment.
932e0d8
to
16bc3ac
Compare
nice work! Is there any way possible we can add some extra logging (not debug) to the console when a test gets snoozed? Since the code now has differing behavior based on time (i.e. tests ran fine yesterday, and today they are failing, but nothing changed!) I think it's important that we mention very loudly when we don't run a test because of the policy/configuration we've set. |
We can move the if len(obj.Arches) > 0 && !hasString(arch, obj.Arches) {
continue
}
if len(stream) > 0 && len(obj.Streams) > 0 && !hasString(stream, obj.Streams) {
continue
}
if obj.SnoozeDate != "" {
snoozeDate, err := time.Parse(snoozeFormat, obj.SnoozeDate)
if err != nil {
return err
} else if today.After(snoozeDate) {
continue
}
fmt.Printf("🕒 Snoozing test until %s \n", snoozeDate.Format("Jan 02 2006"))
}
fmt.Printf("⚠️ Skipping kola test pattern \"%s\":\n", obj.Pattern)
fmt.Printf(" 👉 %s\n", obj.Tracker)
DenylistedTests = append(DenylistedTests, obj.Pattern) And the output will look like:
|
Looks mostly good. Maybe just add the test name in there too just to
|
16bc3ac
to
9e4a764
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.
Cool stuff! Some minor comments, but LGTM overall.
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.
LGTM
731974d
to
ffff0d7
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.
LGTM! Optional nits only, happy to defer.
Added an option to snooze tests until a given date. This snooze date can be specified in kola-denylist.yaml with key "snooze" and format YYYY-MM-DD.
ffff0d7
to
968234b
Compare
968234b
to
e7b7ea3
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.
LGTM! Will let someone else have a final look and press the button.
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.
LGTM - Nice work @saqibali-2k
Add snooze support for tests as described here.