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

Add failed logic for trx logger when TreatNoTestAsError is set to true #2707

Closed
Sanan07 opened this issue Jan 20, 2021 · 9 comments · Fixed by #2758
Closed

Add failed logic for trx logger when TreatNoTestAsError is set to true #2707

Sanan07 opened this issue Jan 20, 2021 · 9 comments · Fixed by #2758
Assignees
Labels
engineering enhancement future Improvements which can be done in future and need team triage

Comments

@Sanan07
Copy link
Contributor

Sanan07 commented Jan 20, 2021

We have TreatNoTestAsError parameter. When it is set to true and no tests discovered/executed, vstest.console.exe returns 1.

We do not see that in the TRX logger, only message about "no tests found".

We can potentially change TestRun/ResultSummary@Outcome to specify it or do the same as usual failed cases.

Current Behavior

<ResultSummary outcome="Completed">
   <Counters total="0" executed="0" passed="0" failed="0" error="0" timeout="0" aborted="0" inconclusive="0" passedButRunAborted="0" notRunnable="0" notExecuted="0" disconnected="0" warning="0" completed="0" inProgress="0" pending="0" />
   <RunInfos>
      <RunInfo computerName="" outcome="Warning" timestamp="2021-01-20T21:07:50.5110319+01:00">
         <Text>No test is available in D:\tmp\notests\bin\Debug\net5.0\.\notests.dll. Make sure that test discoverer &amp; executors are registered and platform &amp; framework version settings are appropriate and try again.</Text>
      </RunInfo>
   </RunInfos>
</ResultSummary>

Proposed Behavior

<ResultSummary outcome="Failed">
   <Counters total="0" executed="0" passed="0" failed="0" error="0" timeout="0" aborted="0" inconclusive="0" passedButRunAborted="0" notRunnable="0" notExecuted="0" disconnected="0" warning="0" completed="0" inProgress="0" pending="0" />
   <RunInfos>
      <RunInfo computerName="" outcome="Error" timestamp="2021-01-20T21:07:50.5110319+01:00">
         <Text>No test is available in D:\tmp\notests\bin\Debug\net5.0\.\notests.dll. Make sure that test discoverer &amp; executors are registered and platform &amp; framework version settings are appropriate and try again.</Text>
      </RunInfo>
   </RunInfos>
</ResultSummary>

(Note that the value of //ResultSummary/RunInfos/RunInfo/@outcome attribute is changed.)

AB#1272812

@Sanan07 Sanan07 added enhancement engineering future Improvements which can be done in future and need team triage labels Jan 20, 2021
@Sanan07 Sanan07 self-assigned this Jan 20, 2021
@nohwnd
Copy link
Member

nohwnd commented Jan 27, 2021

IIRC this being just a warning was one of the motivations to introduce the new option. That way the build actually fails if there are no tests. Maybe the TRX should reflect that and it should be an error instead of warning?

@Sanan07
Copy link
Contributor Author

Sanan07 commented Jan 27, 2021

Yes, that is the point of this : to change trx to show outcome as "failed" instead of "warning".

@nohwnd
Copy link
Member

nohwnd commented Jan 27, 2021

I thought the example above is an example of the final message, and the example above says warning. But if there will be error then great. 🙂

@Haplois
Copy link
Contributor

Haplois commented Jan 28, 2021

Should we keep the //ResultSummary/@outcome attribute as Completed in this case? Since the test run actually never happened. Shouldn't we set that as Failed as well?

@Sanan07
Copy link
Contributor Author

Sanan07 commented Jan 28, 2021

We can provide new outcome like Not started. It is not Failed, cause there were not errors during discovery or execution.

@nohwnd
Copy link
Member

nohwnd commented Jan 28, 2021

But how will that show in AzDO? IF that is a new status, AzDO won't know about not started and because TRX is versionless we won't be able to tell them that this is supported. Also they would have to do development to support this.

@Sanan07
Copy link
Contributor Author

Sanan07 commented Jan 28, 2021

Yes, you are right. IMHO, in this case Completed could be more reasonable, cause we finished test run even if test execution did not start, while Failed means that something wrong happened during discovery/ execution.

@Haplois
Copy link
Contributor

Haplois commented Jan 28, 2021

Currently, even one test fails, we report //ResultSummary/@outcome as Failed. Considering we report a failure from vstest.console.exe, I think we should report a failure in this case as well. Otherwise, it implies a successful run. If we have the TreatNoTestAsError switch set, we should treat this as a failure.

@Sanan07
Copy link
Contributor Author

Sanan07 commented Jan 28, 2021

Thanks, I will take into consideration these opinions, while working on this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engineering enhancement future Improvements which can be done in future and need team triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants