testing: sort completed tests by start sequence, not completion time (#235759)

* testing: sort completed tests by start sequence, not completion time

Fixes #235667

The root cause of the issue is the separate test run that playwright does:

https://github.com/microsoft/playwright-vscode/blob/4fba0d0873ee9bf5de17219fa2f48201fd16162f/src/extension.ts#L403-L422.

VS Code by default only shows failure messages from the last-ended test
run, which was the original (unused) test run VS Code created, because
it ends automatically after the runHandler's promise resolves. A good
change to make, which happens to fix this bug, is ensuring results are
retained in the order the user started the test runs versus the order
in which they ended.

Note that playwright is able to avoid the duplicate run since #213182,
but I think this is still a sensible change to make.

* fix other test

Author: connor4312

src/vs/workbench/contrib/testing/common/testResult.ts

@@ -340,6 +340,7 @@ export class LiveTestResult extends Disposable implements ITestResult {
 		public readonly id: string,
 		public readonly persist: boolean,
 		public readonly request: ResolvedTestRunRequest,
+		public readonly insertOrder: number,
 		@ITelemetryService private readonly telemetry: ITelemetryService,
 	) {
 		super();

src/vs/workbench/contrib/testing/common/testResultService.ts

@@ -78,6 +78,7 @@ export class TestResultService extends Disposable implements ITestResultService
 	private _results: ITestResult[] = [];
 	private readonly _resultsDisposables: DisposableStore[] = [];
 	private testChangeEmitter = this._register(new Emitter<TestResultItemChange>());
+	private insertOrderCounter = 0;
 
 	/**
 	 * @inheritdoc
@@ -139,7 +140,7 @@ export class TestResultService extends Disposable implements ITestResultService
 	public createLiveResult(req: ResolvedTestRunRequest | ExtensionRunTestsRequest) {
 		if ('targets' in req) {
 			const id = generateUuid();
-			return this.push(new LiveTestResult(id, true, req, this.telemetryService));
+			return this.push(new LiveTestResult(id, true, req, this.insertOrderCounter++, this.telemetryService));
 		}
 
 		let profile: ITestRunProfile | undefined;
@@ -164,7 +165,7 @@ export class TestResultService extends Disposable implements ITestResultService
 			});
 		}
 
-		return this.push(new LiveTestResult(req.id, req.persist, resolved, this.telemetryService));
+		return this.push(new LiveTestResult(req.id, req.persist, resolved, this.insertOrderCounter++, this.telemetryService));
 	}
 
 	/**
@@ -251,7 +252,17 @@ export class TestResultService extends Disposable implements ITestResultService
 	}
 
 	private resort() {
-		this.results.sort((a, b) => (b.completedAt ?? Number.MAX_SAFE_INTEGER) - (a.completedAt ?? Number.MAX_SAFE_INTEGER));
+		this.results.sort((a, b) => {
+			// Running tests should always be sorted higher:
+			if (!!a.completedAt !== !!b.completedAt) {
+				return a.completedAt === undefined ? -1 : 1;
+			}
+
+			// Otherwise sort by insertion order, hydrated tests are always last:
+			const aComp = a instanceof LiveTestResult ? a.insertOrder : -1;
+			const bComp = b instanceof LiveTestResult ? b.insertOrder : -1;
+			return bComp - aComp;
+		});
 	}
 
 	private updateIsRunning() {

src/vs/workbench/contrib/testing/test/common/testResultService.test.ts

@@ -40,13 +40,15 @@ suite('Workbench - Test Results Service', () => {
 		}]
 	});
 
+	let insertCounter = 0;
+
 	class TestLiveTestResult extends LiveTestResult {
 		constructor(
 			id: string,
 			persist: boolean,
 			request: ResolvedTestRunRequest,
 		) {
-			super(id, persist, request, NullTelemetryService);
+			super(id, persist, request, insertCounter++, NullTelemetryService);
 			ds.add(this);
 		}
 
@@ -263,27 +265,29 @@ suite('Workbench - Test Results Service', () => {
 				'',
 				false,
 				defaultOpts([]),
+				insertCounter++,
 				NullTelemetryService,
 			));
 			results.clear();
 
 			assert.deepStrictEqual(results.results, [r2]);
 		});
 
-		test('keeps ongoing tests on top', async () => {
+		test('keeps ongoing tests on top, restored order when done', async () => {
 			results.push(r);
 			const r2 = results.push(new LiveTestResult(
 				'',
 				false,
 				defaultOpts([]),
+				insertCounter++,
 				NullTelemetryService,
 			));
 
 			assert.deepStrictEqual(results.results, [r2, r]);
 			r2.markComplete();
 			assert.deepStrictEqual(results.results, [r, r2]);
 			r.markComplete();
-			assert.deepStrictEqual(results.results, [r, r2]);
+			assert.deepStrictEqual(results.results, [r2, r]);
 		});
 
 		const makeHydrated = async (completedAt = 42, state = TestResultState.Passed) => new HydratedTestResult({

src/vs/workbench/contrib/testing/test/common/testResultStorage.test.ts

@@ -25,6 +25,7 @@ suite('Workbench - Test Result Storage', () => {
 			'',
 			true,
 			{ targets: [], group: TestRunProfileBitset.Run },
+			1,
 			NullTelemetryService,
 		));