Skip to content

Commit 3e59d46

Browse files
authored
testing: sort completed tests by start sequence, not completion time (microsoft#235758)
* testing: sort completed tests by start sequence, not completion time Fixes microsoft#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 microsoft#213182, but I think this is still a sensible change to make. * fix other test
1 parent 61f2c67 commit 3e59d46

File tree

4 files changed

+23
-6
lines changed

4 files changed

+23
-6
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ export class LiveTestResult extends Disposable implements ITestResult {
340340
public readonly id: string,
341341
public readonly persist: boolean,
342342
public readonly request: ResolvedTestRunRequest,
343+
public readonly insertOrder: number,
343344
@ITelemetryService private readonly telemetry: ITelemetryService,
344345
) {
345346
super();

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ export class TestResultService extends Disposable implements ITestResultService
7878
private _results: ITestResult[] = [];
7979
private readonly _resultsDisposables: DisposableStore[] = [];
8080
private testChangeEmitter = this._register(new Emitter<TestResultItemChange>());
81+
private insertOrderCounter = 0;
8182

8283
/**
8384
* @inheritdoc
@@ -139,7 +140,7 @@ export class TestResultService extends Disposable implements ITestResultService
139140
public createLiveResult(req: ResolvedTestRunRequest | ExtensionRunTestsRequest) {
140141
if ('targets' in req) {
141142
const id = generateUuid();
142-
return this.push(new LiveTestResult(id, true, req, this.telemetryService));
143+
return this.push(new LiveTestResult(id, true, req, this.insertOrderCounter++, this.telemetryService));
143144
}
144145

145146
let profile: ITestRunProfile | undefined;
@@ -164,7 +165,7 @@ export class TestResultService extends Disposable implements ITestResultService
164165
});
165166
}
166167

167-
return this.push(new LiveTestResult(req.id, req.persist, resolved, this.telemetryService));
168+
return this.push(new LiveTestResult(req.id, req.persist, resolved, this.insertOrderCounter++, this.telemetryService));
168169
}
169170

170171
/**
@@ -251,7 +252,17 @@ export class TestResultService extends Disposable implements ITestResultService
251252
}
252253

253254
private resort() {
254-
this.results.sort((a, b) => (b.completedAt ?? Number.MAX_SAFE_INTEGER) - (a.completedAt ?? Number.MAX_SAFE_INTEGER));
255+
this.results.sort((a, b) => {
256+
// Running tests should always be sorted higher:
257+
if (!!a.completedAt !== !!b.completedAt) {
258+
return a.completedAt === undefined ? -1 : 1;
259+
}
260+
261+
// Otherwise sort by insertion order, hydrated tests are always last:
262+
const aComp = a instanceof LiveTestResult ? a.insertOrder : -1;
263+
const bComp = b instanceof LiveTestResult ? b.insertOrder : -1;
264+
return bComp - aComp;
265+
});
255266
}
256267

257268
private updateIsRunning() {

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,15 @@ suite('Workbench - Test Results Service', () => {
4040
}]
4141
});
4242

43+
let insertCounter = 0;
44+
4345
class TestLiveTestResult extends LiveTestResult {
4446
constructor(
4547
id: string,
4648
persist: boolean,
4749
request: ResolvedTestRunRequest,
4850
) {
49-
super(id, persist, request, NullTelemetryService);
51+
super(id, persist, request, insertCounter++, NullTelemetryService);
5052
ds.add(this);
5153
}
5254

@@ -263,27 +265,29 @@ suite('Workbench - Test Results Service', () => {
263265
'',
264266
false,
265267
defaultOpts([]),
268+
insertCounter++,
266269
NullTelemetryService,
267270
));
268271
results.clear();
269272

270273
assert.deepStrictEqual(results.results, [r2]);
271274
});
272275

273-
test('keeps ongoing tests on top', async () => {
276+
test('keeps ongoing tests on top, restored order when done', async () => {
274277
results.push(r);
275278
const r2 = results.push(new LiveTestResult(
276279
'',
277280
false,
278281
defaultOpts([]),
282+
insertCounter++,
279283
NullTelemetryService,
280284
));
281285

282286
assert.deepStrictEqual(results.results, [r2, r]);
283287
r2.markComplete();
284288
assert.deepStrictEqual(results.results, [r, r2]);
285289
r.markComplete();
286-
assert.deepStrictEqual(results.results, [r, r2]);
290+
assert.deepStrictEqual(results.results, [r2, r]);
287291
});
288292

289293
const makeHydrated = async (completedAt = 42, state = TestResultState.Passed) => new HydratedTestResult({

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ suite('Workbench - Test Result Storage', () => {
2525
'',
2626
true,
2727
{ targets: [], group: TestRunProfileBitset.Run },
28+
1,
2829
NullTelemetryService,
2930
));
3031

0 commit comments

Comments
 (0)