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

Tests that are timing out are flagged as failed instead of timed out #4139

Closed
dotMorten opened this issue Jul 31, 2018 · 8 comments
Closed

Comments

@dotMorten
Copy link
Contributor

Description

When a test times out, there's no indication in the test report it timed out - it's showing as failed instead. There's quite a big difference between the two, as a timed out test is more of the inconclusive kind.

I believe this issue is occurring here:

https://github.com/Microsoft/testfx/blob/fa7f9a8ecb2fc01fd68745a467a977e0485d6320/src/Adapter/MSTest.CoreAdapter/Extensions/TestResultExtensions.cs#L24-L26

In line 24, the outcome is correctly reported as Timeout, but the following if-condition is finding the exception and then sets the error to Failed.

Alternatively the issue is that the TestFailureException shouldn't have been set in the first place, since that error message doesn't provide any sort of extra value anyway:

https://github.com/Microsoft/testfx/blob/fa7f9a8ecb2fc01fd68745a467a977e0485d6320/src/Adapter/MSTest.CoreAdapter/Execution/TestMethodInfo.cs#L704-L706

@cltshivash cltshivash added the bug label Aug 3, 2018
@pvlakshm pvlakshm added enhancement and removed bug labels Aug 3, 2018
@pvlakshm
Copy link
Contributor

pvlakshm commented Aug 3, 2018

This is how it has behaved since MSTestV1.
Hence this will need to be treated as an enhancement.
Clearing away the bug field.
@dotMorten, would you like to test and submit a patch?

@dotMorten
Copy link
Contributor Author

I'd be happy to. While I think the second option is more correct (no error message) and simpler to do, the first option that keeps the message but sets the right status is probably the least behavioral change as only the status and not the error message would change.
Agreed?

@dotMorten
Copy link
Contributor Author

After a closer look, I'm realizing that this would require a change in the Test ObjectModel, since it doesn't contain that enum:
https://github.com/Microsoft/vstest/blob/abc7599fe09f22bcea35a707d3b9c94a63e6ee88/src/Microsoft.TestPlatform.ObjectModel/TestOutcome.cs#L11

@pvlakshm
Copy link
Contributor

pvlakshm commented Aug 9, 2018

Adding @singhsarab to comment on any changes to the OM.

@singhsarab
Copy link
Contributor

Sorry, I missed this thread.

There are a few reasons I am not very keen to take this

  1. We have not received an ask for this until now and users are used to this behavior. From Testplatform stand point, anything that results in unexpected behavior is treated as failure and the test outcomes are generic enough to fit the needs of all the adapters.
  2. Changing the OM is not easy. Even if I add a new TestOutcome, and map it in the adapter, adapter can be loaded against the older, already shipped, versions of OM.

@dotMorten
Copy link
Contributor Author

Well here's an alternative solution:
The TRX format has timeout in the schema but it's never used. What if this just would output things correctly?

@Evangelink Evangelink transferred this issue from microsoft/testfx Nov 24, 2022
@Evangelink
Copy link
Member

Evangelink commented Nov 24, 2022

Moved to vstest repository as we are limited by the enum provided by Test Platform Object Model as pointed out by @dotMorten.

@nohwnd
Copy link
Member

nohwnd commented Jul 9, 2024

This is a new feature and won't be implemented, we are focusing on adding new features to Testing.Platform instead. https://aka.ms/testingplatform

@nohwnd nohwnd closed this as not planned Won't fix, can't repro, duplicate, stale Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants