-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Add a separate test pass to run flaky tests that doesn't fail the build #8486
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
Changes from all commits
a988f6e
598bd30
a5eddf1
a12084b
c33ad15
39c8e6f
c5e622f
fe6c61d
6cc5b8e
b77353f
db96c1d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<Project> | ||
<!-- Override where xUnit logs and results go if we're doing the flaky run --> | ||
<PropertyGroup Condition="'$(RunFlakyTests)' == 'true'"> | ||
<ArtifactsLogDir>$(ArtifactsDir)log\$(Configuration)\Flaky\</ArtifactsLogDir> | ||
<ArtifactsTestResultsDir>$(ArtifactsDir)TestResults\$(Configuration)\Flaky\</ArtifactsTestResultsDir> | ||
</PropertyGroup> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<Project> | ||
<!-- Local Dev Flakiness --> | ||
<PropertyGroup> | ||
<_FlakyRunAdditionalArgs>-trait "Flaky:All=true"</_FlakyRunAdditionalArgs> | ||
<_NonFlakyRunAdditionalArgs>-notrait "Flaky:All=true"</_NonFlakyRunAdditionalArgs> | ||
</PropertyGroup> | ||
|
||
<!-- Azure Pipelines Flakiness --> | ||
<PropertyGroup Condition="'$(AGENT_OS)' != ''"> | ||
<_FlakyRunAdditionalArgs>$(_FlakyRunAdditionalArgs) -trait "Flaky:AzP:All=true" -trait "Flaky:AzP:OS:$(AGENT_OS)=true"</_FlakyRunAdditionalArgs> | ||
<_NonFlakyRunAdditionalArgs>$(_NonFlakyRunAdditionalArgs) -notrait "Flaky:AzP:All=true" -notrait "Flaky:AzP:OS:$(AGENT_OS)=true"</_NonFlakyRunAdditionalArgs> | ||
</PropertyGroup> | ||
|
||
<PropertyGroup> | ||
<TestRunnerAdditionalArguments Condition="'$(RunFlakyTests)' == ''">$(_NonFlakyRunAdditionalArgs)</TestRunnerAdditionalArguments> | ||
<TestRunnerAdditionalArguments Condition="'$(RunFlakyTests)' == 'true'">$(_FlakyRunAdditionalArgs)</TestRunnerAdditionalArguments> | ||
</PropertyGroup> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,6 @@ | ||
REM Disable "!Foo!" expansions because they break the filter syntax | ||
setlocal disableextensions | ||
|
||
set target=%1 | ||
set sdkVersion=%2 | ||
set runtimeVersion=%3 | ||
|
@@ -18,12 +21,35 @@ set HELIX=%helixQueue% | |
|
||
%DOTNET_ROOT%\dotnet vstest %target% -lt >discovered.txt | ||
find /c "Exception thrown" discovered.txt | ||
if %errorlevel% equ 0 ( | ||
echo Exception thrown during test discovery. | ||
type discovered.txt | ||
REM "ERRORLEVEL is not %ERRORLEVEL%" https://blogs.msdn.microsoft.com/oldnewthing/20080926-00/?p=20743/ | ||
if not errorlevel 1 ( | ||
echo Exception thrown during test discovery. 1>&2 | ||
type discovered.txt 1>&2 | ||
exit 1 | ||
) | ||
|
||
%DOTNET_ROOT%\dotnet vstest %target% --logger:trx --logger:console;verbosity=normal | ||
set exit_code=0 | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "provided in" what? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shhhhhh |
||
REM Filter syntax: https://github.com/Microsoft/vstest-docs/blob/master/docs/filter.md | ||
set NONFLAKY_FILTER="Flaky:All!=true&Flaky:Helix:All!=true&Flaky:Helix:Queue:All!=true&Flaky:Helix:Queue:%HELIX%!=true" | ||
echo Running non-flaky tests. | ||
%DOTNET_ROOT%\dotnet vstest %target% --logger:trx --logger:console;verbosity=normal --TestCaseFilter:%NONFLAKY_FILTER% | ||
if errorlevel 1 ( | ||
echo Failure in non-flaky test 1>&2 | ||
set exit_code=1 | ||
REM DO NOT EXIT | ||
) | ||
|
||
set FLAKY_FILTER="Flaky:All=true|Flaky:Helix:All=true|Flaky:Helix:Queue:All=true|Flaky:Helix:Queue:%HELIX%=true" | ||
echo Running known-flaky tests. | ||
%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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest including similar comments in runtests.sh |
||
) | ||
|
||
exit %exit_code% | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
version:3.0.0-build-20190306.2 | ||
commithash:18c06e0b774622c87560e6f21b97e099307fd10c | ||
version:3.0.0-build-20190314.2 | ||
commithash:e3a8a2aae198f1ef26309714ccba6835be2437c3 |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Still working on Unix scripts btw)
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.