---

This section details on the original issue you should resolve

<issue_title>Cancelled tests not reported as failures</issue_title>
<issue_description>See this run: https://github.com/thomhurst/ModularPipelines/actions/runs/12072550368/job/33666771665?pr=787#step:9:3473

Cancelled tests are marked as failures.

I'd argue they shouldn't be counted as failures as that's misleading. They haven't failed per se, they were just terminated.

If the overall test run was cancelled it should fail overall, but not each cancelled test imo.

What do you think?</issue_description>

<agent_instructions>Obsolete the cancelled test node state.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@Evangelink Is this functionally expected to have a test being cancelled? For me, it is an error even if that's the result of a user requesting cancellation.

We cannot say the test is ok as it didn't reach the end of its action.</comment_new>
<comment_new>@Evangelink
I understand the TE situation, let's put it on the side for a second to focus on CLI experience, what would be your expectations there?</comment_new>
<comment_new>@nohwnd
Sounds reasonable to me to not report it as error (there is no additional info in the error anyway?) as long as we guarantee that the whole run will finish with error code.

@Youssef1313 > The first failure will cancel the test session's cancellation token, and so all pending tests would then be marked as cancelled. Sifting through the output/test explorer to find that failure is harder if all the cancellations look like failures too.

Why not simply skip reporting any test results for all the pending tests in this case?</comment_new>
<comment_new>@Youssef1313
My thought after revisiting this, I feel like we shouldn't have introduced a cancelled test node state altogether.

If cancellation is requested (e.g, via Ctrl+C), the test framework should just call cancellationToken.ThrowIfCancellationRequested(), which then propagates up as cancellation.

@thomhurst Is there any reason you really need this state? If others agree, I would personally consider marking it as obsolete.

• We don't use this state in the VSTestBridge (so both MSTest and NUnit don't use it).
• xUnit.net doesn't seem to also use that state (cc @bradwilson for confirmation)
• I don't see exactly how this is reachable in TUnit. I see usage here, but I don't see anything that sets TestState.Cancelled. @thomhurst Can you clarify please how you are using that? There is another usage here, but it says it shouldn't happen (should you actually throw UnreachableException so it becomes more clear this is not supposed to be reached?)
• MSTest.Engine (the experimental source gen based MSTest is using it, but I don't think it's correct to use that state).</comment_new>
  <comment_new>@Youssef1313
  For timeout, it makes sense to me to end up as a failure.
  For cancelled, the whole state, IMO, shouldn't have existed. I'm trying to understand more how/why you need a cancelled state. When a test run is cancelled, I think you should just throw and let the flow back to MTP to finalize any needed cleanups and exit with aborted exit code.</comment_new>
  <comment_new>@Youssef1313

In regards to throwing and bubbling back to MTP, that doesn't really work. You get unclear errors like Test framework reported a start event but not an end event (or something like that, can't remember off the top of my head.)

Can you have more clear repro of what goes wrong when you throw cancellation and let the flow back to MTP? I would prefer to address that instead, and then obsolete the cancelled state.</comment_new>
<comment_new>@Evangelink
MTP currently catches cancellation exceptions if the token on the exception matches the token of MTP execution be wise that's the only way to be sure this is a cancellation requested by MTP. Your example although throwing Oce is not something triggered by MTP so it's considered like any other exception.</comment_new>
<comment_new>@Youssef1313
Exactly. Cancellation should happen via cancellationToken.ThrowIfCancellationRequested(), and only using the cancellationToken that MTP passes to the framework.</comment_new>