Skip to content

Read E2E logs and fail the job if no tests matched the filter #62222

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

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Jun 3, 2025

Trying to avoid the situation we faced with #62223

Description

For a few days we were merging PRs that did not run through all the tests. If we had a mechanism that would check the test output for "No test matches the given testcase filter" and would fail the job, we would have caught it earlier.

I am aware that not only E2E tests were affected, template tests or generally, tests running on Helix also had this problem, I don't see an easy way how to use a similar mechanism there.

@ilonatommy ilonatommy self-assigned this Jun 3, 2025
@github-actions github-actions bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jun 3, 2025
@ilonatommy ilonatommy added the * NO MERGE * Do not merge this PR as long as this label is present. label Jun 3, 2025
@ilonatommy
Copy link
Member Author

ilonatommy commented Jun 3, 2025

The alternative way of running triggered the test process. We got an error informing about missing xunit.runner.console package:
artifacts

The application to execute does not exist: '/home/vsts/work/1/s/.packages/xunit.runner.console/2.9.2/tools/netcoreapp2.0/xunit.console.dll'
=== COMMAND LINE ===
"/home/vsts/work/1/s/.dotnet/dotnet" exec --depsfile "/home/vsts/work/1/s/src/Components/test/E2ETest/bin/Release/net10.0/Microsoft.AspNetCore.Components.E2ETests.deps.json" --runtimeconfig "/home/vsts/work/1/s/src/Components/test/E2ETest/bin/Release/net10.0/Microsoft.AspNetCore.Components.E2ETests.runtimeconfig.json"  "/home/vsts/work/1/s/.packages/xunit.runner.console/2.9.2/tools/netcoreapp2.0/xunit.console.dll" "/home/vsts/work/1/s/src/Components/test/E2ETest/bin/Release/net10.0/Microsoft.AspNetCore.Components.E2ETests.dll" -noautoreporters -xml "/home/vsts/work/1/s/artifacts/TestResults/Release/Microsoft.AspNetCore.Components.E2ETests_net10.0_x64.xml" -html "/home/vsts/work/1/s/artifacts/TestResults/Release/Microsoft.AspNetCore.Components.E2ETests_net10.0_x64.html" -notrait "Quarantined=true"  -nocolor > "/home/vsts/work/1/s/artifacts/log/Release/Microsoft.AspNetCore.Components.E2ETests_net10.0_x64.log" 2>&1

Locally we don't experience these problems using eng/build.sh command.

@ilonatommy ilonatommy changed the title [WIP] Experiment if build.sh -test fails the same way dotnet test does [WIP] Find workaround for dotnet test failing to discover tests Jun 3, 2025
@ilonatommy
Copy link
Member Author

The binlog of dotnet test does not present any abnormalities in comparison the the passing binlog.
The test results contain:
[xUnit.net 00:00:00.17] Skipping: Microsoft.AspNetCore.Components.E2ETests (no reference to xUnit.net)

artifacts

@ilonatommy ilonatommy changed the title [WIP] Find workaround for dotnet test failing to discover tests Read E2E logs and fail the job if no tests matched the filter Jun 5, 2025
@ilonatommy ilonatommy marked this pull request as ready for review June 5, 2025 09:03
@ilonatommy ilonatommy requested review from wtgodbe and a team as code owners June 5, 2025 09:03
@ilonatommy ilonatommy removed the * NO MERGE * Do not merge this PR as long as this label is present. label Jun 5, 2025
@ilonatommy
Copy link
Member Author

This PR's CI is supposed to fail, to show that the mechanism is working. After merging #62250 it should go green.

Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, migrate to xunit.v3+MTP where zero tests ran is error by default :)

@ilonatommy
Copy link
Member Author

Or, migrate to xunit.v3+MTP where zero tests ran is error by default :)

This is an interesting point. Is #62250 doing that? Does "arcade's default" mean v3?

@ilonatommy
Copy link
Member Author

ilonatommy commented Jun 5, 2025

Or, migrate to xunit.v3+MTP where zero tests ran is error by default :)

This is an interesting point. Is #62250 doing that? Does "arcade's default" mean v3?

Yes, it does:
https://github.com/dotnet/arcade/blob/d3a9e0bf843815be1a2b30895e2060ef73735772/Directory.Packages.props#L20

in this situation, I don't see a point in reading the logs if the newer version will fail it.

@Youssef1313
Copy link
Member

Or, migrate to xunit.v3+MTP where zero tests ran is error by default :)

This is an interesting point. Is #62250 doing that? Does "arcade's default" mean v3?

Nope. My PR only uses xunit.runner.visualstudio 3.x, but it's still xunit 2.x.
The update to xunit.v3 is usually more involved.

But happy to help you with the migration. It was done for Aspire (dotnet/aspire#8403 and dotnet/aspire#8498), and in progress for winforms (dotnet/winforms#13540). We also did it for Uno Platform (https://platform.uno/blog/microsoft-contributes-to-uno-platform-dramatically-reducing-testing-time/), and many more internal (and external) repos.

@DeagleGross
Copy link
Member

This PR's CI is supposed to fail, to show that the mechanism is working. After merging #62250 it should go green.

it works!
image

@ilonatommy
Copy link
Member Author

@lewing, what are your thoughts about xunit upgrade? Should we go for it right now or rather grep for now and log that as a future work?

@ilonatommy ilonatommy enabled auto-merge (squash) June 6, 2025 08:21
@ilonatommy
Copy link
Member Author

Threshold working as well:
image

Setting auto merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants