Fix #142028

[Stefan Haller (stefanhaller)]

If a test is run multiple times during a single test run, don't allow to set
the state back to a lower one, e.g. if the first test run failed but the second
passed. See #142028 for a reproduction recipe.

Fixes #142028

[Stefan Haller (stefanhaller)]

Note that this also prevents the state from going back to running if it is passed or failed. This is not ideal, but it's necessary to solve the issue. In the concrete scenario in the linked issue, the sequence of states is actually running - failed - running - passed.

[Connor Peet (connor4312)]

Hm, previously we didn't prescribe meaning to the order of states in the enum. Maybe instead we just say errored, failed, and passed are terminal states and then don't update anything after a terminal state is reached?

[Stefan Haller (stefanhaller)]

That alone wouldn't fix the problem; we need a "stronger" final state to override a "weaker" one. For example, setting passed and then failed needs to result in failed. Setting failed and then passed also needs to result in failed.

So what we'd have to do is something like this:

  if (old state is terminal and
    (new state is not terminal or new state is "weaker" than old state)) {
       // ignore the update
    }
  }

Does that make sense?

I'm a bit unclear how "skipped" plays into this; but I think skipped should be a terminal state too, and come between passed and failed. (Setting passed/skipped or skipped/passed should result in skipped, but setting failed/skipped or skipped/failed should result in failed.) It's a little academic and I think it can't happen in practice, at least not with the C++ extension, so I don't have a strong opinion on this.

Is it possible to change the order of the enum entries? That would make the implementation a bit easier, if we could put the terminal states at the end, and in the right order.

[Stefan Haller (stefanhaller)]

I pushed a new version that does this without reordering the enum entries. I'm not proud of the code, but I'm not an experienced Typescript coder. Let me know what you think.

[Connor Peet (connor4312)]

I think this overall looks good, can you move this object into testingStates.ts?

[Stefan Haller (stefanhaller)]

Done, I force-pushed a new version. I decided to rename it to terminalStatePriorities, that's a little more precise.

[Connor Peet (connor4312)]

Thanks for the PR!