How to ask mstest to stop once it finds a discovery failure like UTA007

[sadjadbp]

Description

We had bunch of test that their signature was async, but return types where void.
So we were receiving errors like this Azure DevOps test step

2018-10-11T06:06:39.0663002Z [MSTest][Discovery][x.dll] UTA007: Method Test1 defined in class Class1 does not have correct signature. Test method marked with the [TestMethod] attribute must be non-static, public, return-type as void and should not take any parameter. Example: public void Test.Class1.Test(). Additionally, if you are using async-await in test method then return-type must be Task. Example: public async Task Test.Class1.Test2()

The question here is what parameter we could pass to vstest.console.exe to break/stop on these errors. We basically didn't know these tests was not running for a long time

[StingyJack]

A similar problem where discovery failure doesnt report as error and tests silently fail to run is apparently correctable by using package reference. Are you using package ref or packages config for your test project?

[sadjadbp]

This is asp.net core project using package ref

[mayankbansal018]

@sadjadbp , currently this by design, both test platform, & framework try to run as many test as possible, until some fatal error occurs.
I assume your expectation over here would that the test adapter throws a warning saying, that following test were not run? Although I'm not sure if it is treating asyn tests which don't return Task, as correct TestMethods

@jayaranigarg, any thoughts on this?

[mayankbansal018]

@sadjadbp , there is no backlog currently to add such a feature to vstest.console, as we intend to run as many test as possible for user, however for such cases users can always generate trx file via passing --logger:trx. The report of this can ve analyzed in vsts, or VS IDE as per user convenience, where an appropriate warning is shown for such methods.

[StingyJack]

@mayankbansal018 - The user intention here is clearly to execute the code marked with [TestMethod] (and without an [Ignore]) as a test, and expects that test to either pass or fail.

It cannot be by design to report a successful overall result when there is a failure to execute any test.

[sadjadbp]

Yes thanks @StingyJack that's exactly my point. Doesn't seems to be a good answer. We didn't know we had a failing/missing test.

[mayankbansal018]

I agree with the user intention to be able to run test not marked as Ignored, but so far the test adapter throws a warning for such adapters, giving user a chance to correct them. Taking a fix here, & failing such tests, could potentially break other customers, who might be aware of it, since adapter has already thrown a warning, & also it is logged in trx files if it is enabled.

I'll talk to team internally, & discuss is it okay to take such a fix, which we can break other customers.
If we are in-fact okay to take a fix, is it possible for one of you take this work, & create a PR?

[sadjadbp]

I'm fine with warning as long as I would have an option to "treat warnings as errors" and that's my original ask here.

[StingyJack]

If we are in-fact okay to take a fix, is it possible for one of you take this work, & create a PR?

@mayankbansal018 - if you can take a fix for the older QualityFramework dll, then possibly. As I've noted in other issues, MSTest2 is not an option for me as it currently requires using nuget PackageReference in order to avoid the "silently skipped tests" issue at topic here.

I'm not sure who would be depending on tests to not be executed and would complain about fixing this. If fixing this breaks someone's code, then that is code that needs to be broken. This is really not something to warn people about, the program cannot comply with the user request ("run all the tests defined"). Its not the same as the program running sub-optimally.

Also a warning about a test adapter (any warning really) is easily missed in any project with more warnings than the build summary or errors list can show in a single page view (I am currently experiencing one type of warning that requires about 10 pages of scrolling in the error list at its default height).

[mayankbansal018]

@sadjadbp @StingyJack , Apologies for getting delayed on this, I've not been able to spend enough time on it, will try to get back within a day or two.

[mayankbansal018]

@jayaranigarg , @cltshivash I was looking into this issue, & it seems a fair ask, that we report test as failed if we cannot run them. E.g. being if a Test is marked as async, but does not have return type Task, we do not run such tests, & just throw a warning.

We do similar thing for tests that are marked as Ignored, & show them as fail as per users input MapInconclusiveToFailed.

On similar lines we can report not run-able as Failed