Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a grace period for new kola tests by adding a creationDate field to the test metadata. For one week following this date, test failures are treated as warnings to prevent blocking CI during initial monitoring. The changes include documentation, metadata structure updates, and logic to manage the WarnOnErrorTests list. One concern was raised regarding the use of filepath.Match for checking WarnOnErrorTests; literal test names containing glob characters might fail to match correctly, potentially bypassing the warning logic.
| if targetMeta.CreationDate != "" { | ||
| today := time.Now() | ||
| testCreationDate, err := time.Parse(dateFormat, targetMeta.CreationDate) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| gracePeriodEnds := testCreationDate.Add(gracePeriod) | ||
| if today.Before(gracePeriodEnds) { | ||
| fmt.Printf("⏳ Kola test \"%s\" is in grace period (created: %s, expires: %s), failures will be warnings.\n", t.Name, targetMeta.CreationDate, gracePeriodEnds.Format(dateFormat)) | ||
| WarnOnErrorTests = append(WarnOnErrorTests, t.Name) | ||
| } | ||
| } |
There was a problem hiding this comment.
Adding a literal test name to WarnOnErrorTests might lead to unexpected behavior if the test name contains characters that are special to filepath.Match (such as *, ?, or [). Since IsWarningOnFailure uses filepath.Match to check for matches, a test name like ext.myproj.test[1] would fail to match itself because [1] would be interpreted as a character class. While kola test names are generally well-behaved, it would be safer to ensure that literal names are matched correctly even if they contain glob characters.
New test will have a one week grace period, where any failures will be logged as a warning, rather than an error.
angelcerveraroldan
force-pushed
the
warning-on-new-test-fail
branch
from
8bf8fa4 to
905c582
Compare
|
The general direction here looks good. A few suggestions:
|
2 participants
New tests often block the CI when they fail.
If we add a creation date for new tets, we can throw a warning rather than a failure for a week after the creation date, so that we can monitor the test in the pipeline without blocking it.