Migrate to xunit.v3 #8403
Migrate to xunit.v3 #8403
Conversation
github-actions
bot
added
the
area-integrations
|
14->28s would be problematic if it's real. Could you maybe do repeat measurement to check whether it's normal variation? |
|
@danmoseley I just looked at another "before" build (https://github.com/dotnet/aspire/actions/runs/14140547808/job/39621122075?pr=8392) and that's 26s. So looks like there is large variance already between runs that wouldn't make measurements here easy/useful. This is probably normal because the number of tests being run in each job is too small already. |
|
@danmoseley For the change in one of the test results, I detailed the reason in the commit message (c325772) |
This test used to pass with VSTest and xunit v2 because the tests were run under testhost.exe. So, DistributedApplicationOptions.Assembly was pointing to testhost. Then ResolveProjectDirectory wouldn't correctly find AppHostProjectPath. In DistributedApplicationBuilder constructor, we were falling back to _innerBuilder.Environment.ContentRootPath because ProjectDirectory is null. With xunit.v3 *or* MTP, generally when the test app is executed as normal executable, we have the right assembly and we are able to resolve project directory correctly
Youssef1313
force-pushed
the
xunit.v3-round-2
branch
from
bb67af0 to
c325772
Compare
|
The test logger.LoggedMessage = () =>
{
// wait for at least 2 logs to be written
if (logger.Logs.Count >= 2)
{
tsc.SetResult();
}
};If we got a third log message, that will crash because the Task already completed. |
Youssef1313
requested review from
radical,
eerhardt,
mitchdenny and
ReubenBond
as code owners
| @@ -33,6 +33,11 @@ | |||
|
|
|||
| <DashboardPublishedArtifactsOutputDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsDir)', 'DashboardArtifacts', '$(Configuration)'))</DashboardPublishedArtifactsOutputDir> | |||
| <WorkloadsPackageSource>$(ArtifactsShippingPackagesDir)</WorkloadsPackageSource> | |||
|
|
|||
| <!-- xUnit2031: Do not use Where clause with Assert.Single --> | |||
There was a problem hiding this comment.
@radical I wanted to avoid extra changes in this PR to keep it minimal. But I think it can be addressed later. I see you are already tracking it now in the follow-up issue.
# Conflicts: # tests/Aspire.Azure.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/EnrichNpgsqlTests.cs
|
@danmoseley @eerhardt @joperezr @RussKie Any objections, or concerns about this? |
|
I have merged main, and fixed the new QuarantineTestAttribute . cc @RussKie |
| @@ -0,0 +1,18 @@ | |||
| <Project> | |||
There was a problem hiding this comment.
@Youssef1313 is there a reason this isn't done at the Arcade level?
There was a problem hiding this comment.
WIP: dotnet/arcade#15671
There was a problem hiding this comment.
But you will need to wait until you are using latest Arcade 10 anyways
|
But do wait for the azdo build to be green too, before merging. |
|
Thanks @radical. I'll merge once all is green (including AzDO build) then. |
|
All is green. I re-confirmed the number of tests between the build in this PR and the previous build are the same on AzDO (8,986). |
Description
Re-creating the PR after some work on Arcade is done to support xunit.v3 extensions.
The number of jobs you have is too large to have a complete comparison. But taking few random samples:
Two runs used for comparison:
NOTE: I'm comparing a single run before vs a single run after. There might already be high variance as every job is running really small number of tests. So numbers are unlikely to be meaningful here.
NOTE 2: There are 3 test failures that need to be looked at. The failures are similar so the root cause is the same. It might be related to how the test assembly is loaded causing working directory to be different or something.
Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?breaking-changetemplate):doc-ideatemplate):