Skip to content
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

17.3.0 Fails in CI run #3939

Closed
nohwnd opened this issue Aug 11, 2022 · 5 comments
Closed

17.3.0 Fails in CI run #3939

nohwnd opened this issue Aug 11, 2022 · 5 comments

Comments

@nohwnd
Copy link
Member

nohwnd commented Aug 11, 2022

TestPlatform 17.3.0 was reported to fail in CI run by #3937 and #3938, the issue seems to b

The issue seems to be that after 17.3.0 introduced the ability to run multiple target frameworks at the same time, we are trying to run .NET Standard libraries which in 17.2.0 would have been excluded.

The workaround (and best practice) is to filter your test dlls down to what you actually want to run.

❌ The default VSTest task pattern is tests.dll, which includes all assemblies with test in name, potentially including resources, and some TestPlatform assemblies

- task: VSTest@2
  inputs:
    testSelector: 'testAssemblies'
    testAssemblyVer2: |
      **\*test*.dll
      !**\*TestAdapter.dll
      !**\obj\**
    searchFolder: '$(System.DefaultWorkingDirectory)'

✔️ Instead you should use a pattern specific to your test assembly naming convention. Typically *Tests.dll, or *.Tests.dll name is used for a dll containing tests. In that case it is sufficient to simply pick that dll from the bin folder:

**\bin\**\*Tests.dll
**\bin\**\*Test.dll

✔️ You can also amend the original pattern to exclude more dlls, hopefully including only your desired test dll:

**\*test*.dll
!**\obj\**
!**\*.resources.dll
!**\*TestAdapter.dll
!**\*Microsoft.*TestPlatform*.dll
!**\*testhost*.dll
!**\testcentric.engine.metadata.dll

✔️ Set VSTEST_DISABLE_MULTI_TFM_RUN=1 to revert to the previous behavior (or close to previous).

@nohwnd nohwnd pinned this issue Aug 11, 2022
nohwnd added a commit to nohwnd/azure-pipelines-tasks that referenced this issue Aug 19, 2022
VSTest2 task specifies a very wide dll matching pattern which
includes many unexpected dlls. This includes all TestPlatform dlls,
test framework dlls, and often
dlls from local nuget package cache.

Instead replace it with pattern that matches dlls from bin folder only,
and matches the common test dll naming pattern such as:

Product.Tests.dll
Product.Test.dll
ProductUnitTests.dll
Product.Unit.Tests.dll

But does not match *TestPlatform*.dll or MSTest.TestFramework.dll.

This leads to more correct test runs, and faster test runs.

Related microsoft/vstest#3939
@nohwnd
Copy link
Member Author

nohwnd commented Aug 19, 2022

A did a bit more investigation on this last week, there are 2 problems, both of which were mitigated before unde the .NET Framework runner (vstest.console.exe) we ship in Microsoft.TestPlatform package, and that is installed by TestPlatformInstaller (or shipped with VS).

This is specific to the .NET Framework runner because the common determined framework was dependent on the target framework of the runner until 17.2. In 17.3 we don't determine a common framework.

  1. .NET Standard dlls are included in the run, and previously they would be given to .NET Framework testhost to execute. The host would not find any test and it would just continue. In the 17.3, they are given to .NET Core testhost which fails to run them because they lack runtimeconfig.json or deps.json.
  2. There are many library dlls that are built for .NETCoreApp. In 17.2 they would fail to load, and would be silently skipped. In 17.3 they are correctly given to .NET Core testhost, which is unable to execute for the same reason as above.

Both cases can be solved by providing just the correct dlls that contain tests, which makes the run both more accurate and faster.

I am working on a fix that would skip .NET Standard dlls, because no runtime provider will match them. The issue 2. cannot be solved, and IMHO it is correct to fail in that case, because in 17.2 it results in silently skipping over possible test assemblies.

@fowl2
Copy link

fowl2 commented Aug 25, 2022

So this has been broken for 2 weeks now, can we get a comment from the team about getting a fix/revert deployed?

Not sure if it should be the Visual Studio (as the deployment vessel) or Dev Ops (microsoft/azure-pipelines-tasks#16772) folk. Responsibility =/= fastest fix.

@nohwnd
Copy link
Member Author

nohwnd commented Aug 26, 2022

@fowl There is 17.3.1 release in process, this is unlikely to hit VS users (at least we did not get any indication of the failure in 17.3.0 release cycle).

This also can't be fully fixed because we no longer ignore all the .NET dlls that are specified whet there is at least 1 .NET Framerowk dll in the mix. Solution is to amend the filters in the pipeline to select the dlls that should actually be tested by using the filter that matches the naming convention.

Hopefully the change for AzDO will be also accepted and make it the default. The default *test* pattern is causing more than this problem.

@nohwnd
Copy link
Member Author

nohwnd commented Aug 26, 2022

There is also VSTEST_DISABLE_MULTI_TFM_RUN environment variable that can be set to 1 to revert to the previous (or close to previous) behavior.

@matsovef
Copy link

I don't know if this is a similiar problem as described above. When changing from 17.2.0 to 17.3.0 we started to get this error:

2022-08-30T08:44:38.2254917Z Testhost process exited with error: Error:
2022-08-30T08:44:38.2255816Z   An assembly specified in the application dependencies manifest (PRI.Solvency.Web.UI.Tests.deps.json) was not found:
2022-08-30T08:44:38.2256217Z     package: 'Antlr4.Runtime', version: '4.6.6'
2022-08-30T08:44:38.2256622Z     path: 'lib/netstandard1.1/Antlr4.Runtime.dll'

The test project (PRI.Solvency.Web.UI.Tests.csproj) lists the application project (PRI.Solvency.Web.UI.csproj) as a dependency and that project in turn references Antlr4.Runtime. Both those projects target .NET 6.0 while all the other projects within the solution target .NET Framework 4.8.

We only inlcude test projects as test assemblies. The dotnet.exe restore command reports no warnings for both projects.

triptijain2112 added a commit to microsoft/azure-pipelines-tasks that referenced this issue Jun 27, 2023
* Make default test pattern more narrow

VSTest2 task specifies a very wide dll matching pattern which
includes many unexpected dlls. This includes all TestPlatform dlls,
test framework dlls, and often
dlls from local nuget package cache.

Instead replace it with pattern that matches dlls from bin folder only,
and matches the common test dll naming pattern such as:

Product.Tests.dll
Product.Test.dll
ProductUnitTests.dll
Product.Unit.Tests.dll

But does not match *TestPlatform*.dll or MSTest.TestFramework.dll.

This leads to more correct test runs, and faster test runs.

Related microsoft/vstest#3939

* Update versions

* Update Tasks/VsTestV2/task.loc.json

* Fix stars in loc pattern

---------

Co-authored-by: triptijain2112 <92331194+triptijain2112@users.noreply.github.com>
Co-authored-by: Vinayak <110326599+vinayakmsft@users.noreply.github.com>
@nohwnd nohwnd unpinned this issue Aug 4, 2023
@nohwnd nohwnd closed this as completed Jul 8, 2024
grokys added a commit to AvaloniaUI/Avalonia that referenced this issue Aug 2, 2024
grokys added a commit to AvaloniaUI/Avalonia that referenced this issue Aug 2, 2024
github-merge-queue bot pushed a commit to AvaloniaUI/Avalonia that referenced this issue Aug 3, 2024
* Record video for failing win32 integration tests.

* Be more specific when selecting the test dll.

microsoft/vstest#3939

---------

Co-authored-by: Julien Lebosquain <julien@lebosquain.net>
maxkatz6 pushed a commit to AvaloniaUI/Avalonia that referenced this issue Aug 3, 2024
* Record video for failing win32 integration tests.

* Be more specific when selecting the test dll.

microsoft/vstest#3939

---------

Co-authored-by: Julien Lebosquain <julien@lebosquain.net>
maxkatz6 pushed a commit to AvaloniaUI/Avalonia that referenced this issue Aug 12, 2024
* Record video for failing win32 integration tests.

* Be more specific when selecting the test dll.

microsoft/vstest#3939

---------

Co-authored-by: Julien Lebosquain <julien@lebosquain.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants