microsoft / testfx

MSTest framework and adapter
MIT License
697 stars 250 forks source link

Command line option `minimum-expected-tests` does not accept 0 #3121

Closed baraJanis closed 1 month ago

baraJanis commented 2 months ago

Hi,

i tried setting --minimum-expected-tests option to 0 because my azure devops pipeline fails with a --filter on zero-tests.

https://devblogs.microsoft.com/dotnet/introducing-ms-test-runner/ mentions this: image

But in the source code it says 0 is invalid: https://github.com/microsoft/testfx/blob/86489a221af6533815034fff6b372777c498d657/src/Platform/Microsoft.Testing.Platform/CommandLine/PlatformCommandLineProvider.cs#L130-L134

For now i added --ignore-exit-code 8 as a workaround. Is there a better way?

Evangelink commented 2 months ago

There is definitely something wrong. I am not sure to understand the use case where you would want to run no test, it would seem like a bug to me.

MarcoRossignoli commented 2 months ago

For now i added --ignore-exit-code 8 as a workaround. Is there a better way?

This is correct.

I think we there's a bug in the documentation. Run 0 tests is unexpected so imo doesn't make a lot of sense have the idea of "minimum" on 0 quantity.

baraJanis commented 2 months ago

Thank you!

SymbioticKilla commented 2 months ago

There is definitely something wrong. I am not sure to understand the use case where you would want to run no test, it would seem like a bug to me.

Hi, we use normally --filter to remove integration tests. With old MSTest works without problem. In our case it is "dotnet test solution.sln with -p:TestingPlatformCommandLineArguments" like here https://github.com/microsoft/testfx/issues/2175 MSTest Runner throws error because integration tests report 0 tests. Actually --filter should ignore them completely?

Evangelink commented 2 months ago

MSTest Runner throws error because integration tests report 0 tests. Actually --filter should ignore them completely?

I am not sure to understand what you mean. With the runner (compared to VSTest) we have made the choice to be strict by default and we do consider running no test as an abnormal situation (this was a recurring complaint in VSTest). If you think this is fine for your situation to request test execution of zero tests, then you should use the --ignore-exit-code.

Let me know if I am missing something.

SymbioticKilla commented 2 months ago

MSTest Runner throws error because integration tests report 0 tests. Actually --filter should ignore them completely?

I am not sure to understand what you mean. With the runner (compared to VSTest) we have made the choice to be strict by default and we do consider running no test as an abnormal situation (this was a recurring complaint in VSTest). If you think this is fine for your situation to request test execution of zero tests, then you should use the --ignore-exit-code.

Let me know if I am missing something.

Thanks for quick answer. Example: Solution has 10 test projects. One of the projects is integration tests project. We want to run only unit tests projects with "dotnet test Solution.sln..." So we use --filter parameter to filter integration tests projects out. Short story: Step 1: Run unit tests from solution Step 2: Run integration tests from solution later in the build pipeline

SymbioticKilla commented 1 month ago

Hey! Do you need more information or solution example?

Evangelink commented 1 month ago

Hi @SymbioticKilla,

Sorry!!! It's clear for us!

We have been discussing internally and we don't think it's interesting to support passing 0 to this parameter. We prefer users to keep ignoring the exit code 8

SymbioticKilla commented 1 month ago

Hi @Evangelink ,

thanks! Seems legit.

And what about --filter option? Does it work as designed, that actually whole namespace(Project) is filtered out, but it shows up in the logs and here we have an exit code 8?

Evangelink commented 1 month ago

Blogpost was fixed.

And what about --filter option? Does it work as designed, that actually whole namespace(Project) is filtered out, but it shows up in the logs and here we have an exit code 8?

Do you have a reproducer for this case? It's not entirely clear to me what you mean. Here is the doc about how to filter https://learn.microsoft.com/dotnet/core/testing/selective-unit-tests?pivots=mstest#mstest-examples

SymbioticKilla commented 1 month ago
dotnet test -p:TestingPlatformCommandLineArguments=" --filter FullyQualifiedName!~TestProjectFilter2" .\TestProjectFilter.sln
  Determining projects to restore...
  All projects are up-to-date for restore.
  TestProjectFilter2 -> C:\Users\user\Documents\TestProjectFilter\TestProjectFilter2\bin\Debug\net8.0\TestProjectFilter2.dll
  TestProjectFilter1 -> C:\Users\user\Documents\TestProjectFilter\TestProjectFilter1\bin\Debug\net8.0\TestProjectFilter1.dll
  Run tests: 'C:\Users\user\Documents\TestProjectFilter\TestProjectFilter1\bin\Debug\net8.0\TestProjectFilter1.dll' [net8.0|x64]
  Run tests: 'C:\Users\user\Documents\TestProjectFilter\TestProjectFilter2\bin\Debug\net8.0\TestProjectFilter2.dll' [net8.0|x64]
  Passed! - Failed: 0, Passed: 0, Skipped: 0, Total: 0, Duration: 87ms
  Passed! - Failed: 0, Passed: 1, Skipped: 0, Total: 1, Duration: 112ms
C:\Users\user\Documents\TestProjectFilter\TestProjectFilter2\bin\Debug\net8.0\TestProjectFilter2.dll : error run failed: Tests failed: 'C:\Users\user\Documents\TestProjectFilter\TestProjec
tFilter2\bin\Debug\net8.0\TestResults\TestProjectFilter2_net8.0_x64.log'

TestProjectFilter.zip

As you can see, it is not possible to filter out the whole project without adding --ignore-exit-code 8

Evangelink commented 1 month ago

Ah right! That's because our new platform (MSTest runner) works on exe level so for this test application instance no test was executed. I am more thorned about this case but I think it boils down to the same reasoning.

cc @MarcoRossignoli

Evangelink commented 1 month ago

@SymbioticKilla For now you will have to stick with the --ignore-exit-code 8 but I think it would be useful to be able to have this setting working at group of projects level instead so I have created a new ticket for that.

I'll move forward by closing this one as the blogpost was updated.