Add a separate test pass to run flaky tests that doesn't fail the build #8486
Conversation
| @@ -1,3 +1,6 @@ | |||
| REM Disable "!Foo!" expansions because they break the filter syntax | |||
| setlocal disableextensions | |||
There was a problem hiding this comment.
cc @HaoK
Even with the planned change to ignore the exit code from helix altogether I'd like to get this change in in some form so that when we do start paying attention to the exit code, we don't start getting known-flaky tests causing failures.
There was a problem hiding this comment.
(Still working on Unix scripts btw)
There was a problem hiding this comment.
Yup, ignoring the exit code is hopefully just a short/medium term thing that gives us some time to get flaky in place
There was a problem hiding this comment.
maybe consider passing in the filters to each script (assuming they are the same in the sh version)
There was a problem hiding this comment.
Yeah, they would be the same. I'll look at that. There can be some shell escaping problems with the syntax so it might be tricky, but I'll give it a shot.
Eilon
added
the
area-infrastructure
|
Do you think its worth separating flaky and non flaky farther apart for helix? Basically instead of running both flaky and non flaky on the same helix item (and just ignoring flaky failures), build two helix work items instead of one per test project, only including the additional run helix flaky work item if the test project has any marked (and the script could just take an parameter saying to always report success for that run). For example: |
|
We could do that, but adding a |
analogrelay
force-pushed
the
anurse/flaky-pass
branch
from
f007cf0 to
6f59bbb
Compare
|
🤞 I think I have the YAML and MSBuild goop in place to run the Flaky test pass separately. |
|
Helix results look good (https://mc.dot.net/#/user/aspnetcore/pr~2Faspnet~2Faspnetcore/ci/20190314.26/workItem/Libuv.FunctionalTests-netcoreapp3.0/wilogs). Helix is reporting success even though flaky tests failed. |
|
I believe this is basically ready for review. I will remove the fake-flaky test I added to Kestrel before merging (feel free to withhold final approval until I do that if you'd like) but the rest should be ready to review. |
eng/helix/vstest/runtests.sh
Outdated
| $DOTNET_ROOT/dotnet vstest $1 --logger:trx | ||
| # Run non-flaky tests first | ||
| # We need to specify all possible Flaky filters that apply to this environment, because the flaky attribute | ||
| # only puts the explicit filter traits the user provided in |
| %DOTNET_ROOT%\dotnet vstest %target% --logger:trx --logger:console;verbosity=normal --TestCaseFilter:%FLAKY_FILTER% | ||
| if errorlevel 1 ( | ||
| echo Failure in flaky test 1>&2 | ||
| REM DO NOT EXIT and DO NOT SET EXIT_CODE to 1 |
There was a problem hiding this comment.
Suggest including similar comments in runtests.sh
|
|
||
| REM Run non-flaky tests first | ||
| REM We need to specify all possible Flaky filters that apply to this environment, because the flaky attribute | ||
| REM only puts the explicit filter traits the user provided in |
There was a problem hiding this comment.
Looks good,
one thing that still nags me is running the non flaky + flaky tests together on helix. There's going to be potentially a lot more noise in the helix logs with all the flaky tests at the end so its going to make it even more painful hunting for the non flaky error messages (compared to a clean helix job with its on mission control link + a flaky helix job with a perma red mission control link)
But hopefully once we get trx support for the error reporting maybe it won't be as much of an issue
|
Basically an alternative to consider doing eventually is two helix jobs
|
|
Yeah, I do agree that it won't be the cleanest results format. I could suppress the console output of the flaky run maybe. We could create separate Helix work items for flaky tests if we thought that was useful but it feels like that will take up a lot more resources than it's worth... |
|
Well its not a big deal right now since we are treating all helix tests as flaky so...there's that... |
analogrelay
force-pushed
the
anurse/flaky-pass
branch
from
9a190b9 to
b77353f
Compare
Similar to dotnet/extensions#1239.
This doesn't actually add the pass yet, just a sample flaky test to use to verify things are working. Sorry Kestrel CODEOWNERS, you're getting pinged on this one because you're the random victim ;). I'll remove reviewers and re-add ones that make sense when this is ready to review.
TODO: