Conversation
Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
…ET 10 Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
… but the MassTransit.ActiveMQ package isn't updated yet Tracked: MassTransit/MassTransit#6133
…ss ambiguous referencing
I feel dirty doing that...
.github/workflows/tests.yaml
Outdated
| - name: Run tests | ||
| run: >- | ||
| dotnet test ${{ github.workspace }}/${{ env.TEST_PROJECT_PATH }} | ||
| dotnet run ${{ github.workspace }}/${{ env.TEST_PROJECT_PATH }} |
There was a problem hiding this comment.
You need to:
- Remove
--logger "console;verbosity=normal". - Replace
--logger trxwith--report-trx(and ensure Microsoft.Testing.Extensions.TrxReport is referenced) - Replace all the blame-related args with
--crashdump,--hangdump, and--hangdump-timeout 7m --results-directoryis changing the meaning a little bit with this PR. Previously it would be relative to current working directory${{ github.workspace }}while with this PR it will be relative to the test executable. So either pass the full path, or keep usingdotnet testwhich will handle the path transformation for you.--collect "XPlat Code Coverage"needs to be replaced with--coverage(and ensure Microsoft.Testing.Extensions.CodeCoverage is referenced)
.github/workflows/tests.yaml
Outdated
| --report-trx | ||
| --report-trx-filename "${{ matrix.name }}-${{ matrix.os }}.trx" | ||
| --ignore-exit-code 8 | ||
| -- RunConfiguration.CollectSourceInformation=true |
Directory.Packages.props
Outdated
| <PackageVersion Include="xunit.runner.visualstudio" Version="2.8.2" /> | ||
| <PackageVersion Include="xunit.extensibility.execution" Version="2.9.3" /> | ||
| <PackageVersion Include="Microsoft.DotNet.XUnitExtensions" Version="9.0.0-beta.24568.1" /> | ||
| <PackageVersion Include="xunit.v3" Version="3.2.0" /> |
There was a problem hiding this comment.
nit: there is now xunit.v3.mtp-v2 package.
| <PackageVersion Include="xunit.v3" Version="3.2.0" /> | ||
| <PackageVersion Include="xunit.v3.assert" Version="3.2.0" /> | ||
| <PackageVersion Include="xunit.v3.extensibility.core" Version="3.2.0" /> | ||
| <PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" /> |
There was a problem hiding this comment.
nit: no longer needed if you don't care about VSTest support.
There was a problem hiding this comment.
| <PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" /> |
tests/.runsettings
Outdated
| <RunConfiguration> | ||
| <!-- Filter out failing (wrong framework, platform, runtime or activeissue) tests --> | ||
| <TestCaseFilter>category!=unsupported-platform</TestCaseFilter> | ||
| <TestCaseFilter>category!=failing</TestCaseFilter> |
There was a problem hiding this comment.
runsettings isn't supported with MTP.
tests/Directory.Build.props
Outdated
| https://learn.microsoft.com/dotnet/core/testing/microsoft-testing-platform-exit-codes --> | ||
| <TestRunnerAdditionalArguments>$(TestRunnerAdditionalArguments) --ignore-exit-code 8</TestRunnerAdditionalArguments> | ||
|
|
||
| <!-- <UseMicrosoftTestingPlatformRunner>true</UseMicrosoftTestingPlatformRunner> --> |
Youssef1313
left a comment
There was a problem hiding this comment.
You need to also update global.json to specify the test runner as MTP, and probably ensure it requires .NET 10.
| // outputs "the sum of 2 and 3 is 5" | ||
| var script2 = ps.AddScript("script2", """ | ||
| & ./scripts/script.ps1 @args | ||
| & ./Scripts/script.ps1 @args |
There was a problem hiding this comment.
I don't think that Powershell is case sensitive (cc @nohwnd)
There was a problem hiding this comment.
This is up to the file system not up to powershell. If this needs to run on linux and the filesystem is case sensitive then ./scripts and ./Scripts are two different paths. On windows we typically don't use case sensitive file systems.
|
|
||
| // Forcing linux only due to: https://github.com/modelcontextprotocol/inspector/issues/893 | ||
| [RequiresLinux] | ||
| [RequiresWindows] |
There was a problem hiding this comment.
I have no idea why I made that change, I'm going to revert it 🤣
Evangelink
left a comment
There was a problem hiding this comment.
@aaronpowell the latest errors are weird, it's complaining about a file that doesn't exist when I pull this branch.
I have also noticed the following missing changes:
- In
global.json, add:
"test": {
"runner": "Microsoft.Testing.Platform"
}- In
Directory.Packages.propsandtests/Directory.Build.propsremove:
coverlet.collector-> not compatible with MTP, instead use MS coverageMicrosoft.NET.Test.Sdk-> VSTest testhost part, no longer needed as you are moving to MTPxunit.runner.visualstudio-> VSTest adapter for xUnit, also no longer needed.
- I'd suggest to be careful with the
--ignore-exit-code 8set on all test projects because you are allowing all projects not to run tests without failure. I don't know if you have some other guard in place but if you don't then you would potentially no longer run any test without noticing.
…/Aspire into xunit-3-upgrade
The PowerShell Runspace doesn't when the app host does, which I think is the underlying cause as to why the tests hang. This now monitors the script resources so that when they complete we force a dispose on them (thus freeing any process they may have used) and once all are completed the Runspace is disposed, releasing it and **I think** that should allow the test host to terminate
This reverts commit 1d1b730.
Xunit 3 upgrade hang fix
Upgrading the tests to use xUnit v3, which is what is used by aspire now too.
Close #961