Skip to content

Commit e1f7492

Browse files
connectdotzconnectdotz
authored
fix "each" block test run issues (#1092)
1 parent 5557e93 commit e1f7492

File tree

8 files changed

+94
-41
lines changed

8 files changed

+94
-41
lines changed

‎src/test-provider/jest-test-run.ts

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export type TestRunProtocol = Pick<
77
'name' | 'enqueued' | 'started' | 'errored' | 'failed' | 'passed' | 'skipped' | 'end'
88
>;
99

10-
export type CreateRun = (name: string) => vscode.TestRun;
10+
export type CreateTestRun = (request: vscode.TestRunRequest, name: string) => vscode.TestRun;
1111
export type EndProcessOption = { pid: string; delay?: number; reason?: string };
1212
export type EndOption = EndProcessOption | { reason: string };
1313
const isEndProcessOption = (arg?: EndOption): arg is EndProcessOption =>
@@ -27,11 +27,13 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol {
2727
private verbose: boolean;
2828
private runCount = 0;
2929
public readonly name: string;
30+
private ignoreSkipped = false;
3031

3132
constructor(
3233
name: string,
3334
private context: JestTestProviderContext,
34-
private createRun: CreateRun
35+
private request: vscode.TestRunRequest,
36+
private createRun: CreateTestRun
3537
) {
3638
this.name = `${this.context.ext.workspace.name}:${name}:${SEQ++}`;
3739
this.output = context.output;
@@ -54,13 +56,19 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol {
5456
}
5557
}
5658
/**
57-
* returns the underlying vscode.TestRun, if existing.
58-
* If no run then create one with this.createRun and return it.
59+
* returns the underlying vscode.TestRun, if no run then create one.
5960
**/
60-
private safeRun(): vscode.TestRun {
61+
private vscodeRun(): vscode.TestRun {
6162
if (!this._run) {
6263
const runName = `${this.name} (${this.runCount++})`;
63-
this._run = this.createRun(runName);
64+
65+
this._run = this.createRun(this.request, runName);
66+
this._run.appendOutput(`\r\nTestRun "${runName}" started\r\n`);
67+
68+
// ignore skipped tests if there are more than one test to run
69+
// this is to prevent the later runs override the previous runs's result
70+
this.ignoreSkipped = this.request.include && this.request.include.length > 1 ? true : false;
71+
6472
if (this.verbose) {
6573
console.log(`[${this.context.ext.workspace.name}] JestTestRun "${runName}": created.`);
6674
}
@@ -70,32 +78,34 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol {
7078

7179
// TestRunProtocol
7280
public enqueued = (test: vscode.TestItem): void => {
73-
this.safeRun().enqueued(test);
81+
this.vscodeRun().enqueued(test);
7482
};
7583
public started = (test: vscode.TestItem): void => {
76-
this.safeRun().started(test);
84+
this.vscodeRun().started(test);
7785
};
7886
public errored = (
7987
test: vscode.TestItem,
8088
message: vscode.TestMessage | readonly vscode.TestMessage[],
8189
duration?: number | undefined
8290
): void => {
8391
const _msg = this.context.ext.settings.runMode.config.showInlineError ? message : [];
84-
this.safeRun().errored(test, _msg, duration);
92+
this.vscodeRun().errored(test, _msg, duration);
8593
};
8694
public failed = (
8795
test: vscode.TestItem,
8896
message: vscode.TestMessage | readonly vscode.TestMessage[],
8997
duration?: number | undefined
9098
): void => {
9199
const _msg = this.context.ext.settings.runMode.config.showInlineError ? message : [];
92-
this.safeRun().failed(test, _msg, duration);
100+
this.vscodeRun().failed(test, _msg, duration);
93101
};
94102
public passed = (test: vscode.TestItem, duration?: number | undefined): void => {
95-
this.safeRun().passed(test, duration);
103+
this.vscodeRun().passed(test, duration);
96104
};
97105
public skipped = (test: vscode.TestItem): void => {
98-
this.safeRun().skipped(test);
106+
if (!this.ignoreSkipped) {
107+
this.vscodeRun().skipped(test);
108+
}
99109
};
100110
public end = (options?: EndOption): void => {
101111
if (!this._run) {
@@ -144,4 +154,8 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol {
144154
console.log(`JestTestRun "${runName}": TestRun ended because: ${options?.reason}.`);
145155
}
146156
};
157+
// set request for next time the underlying run needed to be created
158+
updateRequest(request: vscode.TestRunRequest): void {
159+
this.request = request;
160+
}
147161
}

‎src/test-provider/test-item-data.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ abstract class TestItemDataBase implements TestItemData, JestRunnable, WithUri {
6464
if (!this.isTestNameResolved()) {
6565
const parent = this.item.parent && this.context.getData(this.item.parent);
6666
if (parent) {
67+
run.end({ reason: 'unresolved parameterized test' });
68+
run.updateRequest(new vscode.TestRunRequest([parent.item]));
6769
return parent.scheduleTest(run, itemCommand);
6870
}
6971
this.context.output.write(`running an unresolved parameterized test might fail`, 'warn');

‎src/test-provider/test-provider-context.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,7 @@ export class JestTestProviderContext {
8484

8585
createTestRun = (request: vscode.TestRunRequest, options?: JestTestRunOptions): JestTestRun => {
8686
const name = options?.name ?? `testRun-${SEQ++}`;
87-
const createRun = (name: string) => {
88-
const vscodeRun = this.controller.createTestRun(request, name);
89-
vscodeRun.appendOutput(`\r\nTestRun "${name}" started\r\n`);
90-
return vscodeRun;
91-
};
92-
93-
return new JestTestRun(name, this, createRun);
87+
return new JestTestRun(name, this, request, this.controller.createTestRun);
9488
};
9589

9690
// tags

‎tests/manual-mocks.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ jest.mock('../src/test-provider/jest-test-run', () => {
2424
write: jest.fn(),
2525
addProcess: jest.fn(),
2626
isClosed: jest.fn(() => false),
27+
updateRequest: jest.fn(),
2728
};
2829
}),
2930
};

‎tests/test-provider/jest-test-runt.test.ts

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ describe('JestTestRun', () => {
1313
let mockContext: any;
1414
let jestRun: JestTestRun;
1515
let mockCreateTestRun: any;
16+
let mockRequest: any;
1617

1718
beforeEach(() => {
1819
mockContext = {
@@ -27,19 +28,22 @@ describe('JestTestRun', () => {
2728
},
2829
},
2930
};
30-
mockCreateTestRun = jest.fn().mockImplementation((name: string) => ({
31-
name,
32-
appendOutput: jest.fn(),
33-
enqueued: jest.fn(),
34-
started: jest.fn(),
35-
errored: jest.fn(),
36-
failed: jest.fn(),
37-
passed: jest.fn(),
38-
skipped: jest.fn(),
39-
end: jest.fn(),
40-
}));
41-
42-
jestRun = new JestTestRun('test', mockContext, mockCreateTestRun);
31+
mockCreateTestRun = jest
32+
.fn()
33+
.mockImplementation((_request: vscode.TestRunRequest, name: string) => ({
34+
name,
35+
appendOutput: jest.fn(),
36+
enqueued: jest.fn(),
37+
started: jest.fn(),
38+
errored: jest.fn(),
39+
failed: jest.fn(),
40+
passed: jest.fn(),
41+
skipped: jest.fn(),
42+
end: jest.fn(),
43+
}));
44+
45+
mockRequest = {};
46+
jestRun = new JestTestRun('test', mockContext, mockRequest, mockCreateTestRun);
4347
});
4448

4549
describe('constructor', () => {
@@ -229,7 +233,7 @@ describe('JestTestRun', () => {
229233
const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
230234
mockContext.ext.settings.debugMode = true;
231235

232-
jestRun = new JestTestRun('test', mockContext, mockCreateTestRun);
236+
jestRun = new JestTestRun('test', mockContext, mockRequest, mockCreateTestRun);
233237
jestRun.addProcess(pid);
234238
expect(mockCreateTestRun).toHaveBeenCalledTimes(0);
235239

@@ -294,7 +298,7 @@ describe('JestTestRun', () => {
294298
const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
295299
mockContext.ext.settings.debugMode = true;
296300

297-
jestRun = new JestTestRun('test', mockContext, mockCreateTestRun);
301+
jestRun = new JestTestRun('test', mockContext, mockRequest, mockCreateTestRun);
298302
jestRun.addProcess(pid);
299303
expect(mockCreateTestRun).toHaveBeenCalledTimes(0);
300304

@@ -313,7 +317,7 @@ describe('JestTestRun', () => {
313317
it('print warning for runs re-created after close: this means the test-run will be splitted into multiple TestRun', () => {
314318
mockContext.ext.settings.debugMode = true;
315319

316-
jestRun = new JestTestRun('test', mockContext, mockCreateTestRun);
320+
jestRun = new JestTestRun('test', mockContext, mockRequest, mockCreateTestRun);
317321

318322
jestRun.started({} as any);
319323
expect(mockCreateTestRun).toHaveBeenCalledTimes(1);
@@ -332,4 +336,36 @@ describe('JestTestRun', () => {
332336
const run2 = mockCreateTestRun.mock.results[1].value;
333337
expect(run1).not.toBe(run2);
334338
});
339+
describe('multi-items run', () => {
340+
it('ignore skipped tests if there are more than one test to run', () => {
341+
mockRequest = { include: ['test1', 'test2'] };
342+
jestRun = new JestTestRun('test', mockContext, mockRequest, mockCreateTestRun);
343+
344+
jestRun.started({} as any);
345+
expect(mockCreateTestRun).toHaveBeenCalledTimes(1);
346+
const run = mockCreateTestRun.mock.results[0].value;
347+
expect(run.started).toHaveBeenCalled();
348+
349+
jestRun.skipped({} as any);
350+
expect(run.skipped).not.toHaveBeenCalled();
351+
});
352+
});
353+
describe('when request changed', () => {
354+
it('the next createTestRnn will use the new request', () => {
355+
jestRun = new JestTestRun('test', mockContext, mockRequest, mockCreateTestRun);
356+
jestRun.started({} as any);
357+
expect(mockCreateTestRun).toHaveBeenCalledTimes(1);
358+
const run1 = mockCreateTestRun.mock.results[0].value;
359+
expect(run1.started).toHaveBeenCalled();
360+
361+
jestRun.end();
362+
expect(run1.end).toHaveBeenCalled();
363+
364+
const newRequest: any = { include: ['test1'] };
365+
jestRun.updateRequest(newRequest);
366+
jestRun.started({} as any);
367+
expect(mockCreateTestRun).toHaveBeenCalledTimes(2);
368+
expect(mockCreateTestRun.mock.calls[1][0]).toBe(newRequest);
369+
});
370+
});
335371
});

‎tests/test-provider/test-item-data.test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -786,21 +786,27 @@ describe('test-item-data', () => {
786786
expect(process.userData.testItem).toBe(folderData.item);
787787
});
788788
describe('if test name is not resolved', () => {
789-
it('if there is a parent block => will execute it instead', () => {
789+
it('will find the parent block that is resolved to execute instead', () => {
790790
const { doc } = createAllTestItems();
791791
const descNode: any = {
792792
fullName: 'a $describe',
793793
attrs: { nonLiteralName: true },
794-
data: {},
794+
childContainers: [],
795+
childData: [],
795796
};
796797
const testNode: any = { fullName: 'a test', attrs: { isGroup: 'yes' }, data: {} };
797798
const descItem = new TestData(context, doc.uri, descNode, doc.item);
798799
const testItem = new TestData(context, doc.uri, testNode, descItem.item);
799800
const jestRun = createTestRun();
800801

801802
testItem.scheduleTest(jestRun);
803+
802804
expect(process.userData.run).toBe(jestRun);
803805
expect(process.userData.testItem.id).toBe(doc.item.id);
806+
807+
expect(jestRun.end).toHaveBeenCalledTimes(2);
808+
expect(jestRun.updateRequest).toHaveBeenCalledTimes(2);
809+
expect(vscode.TestRunRequest).toHaveBeenLastCalledWith([doc.item]);
804810
});
805811
it('if failed to get parent block, will attempt to run the test anyway', () => {
806812
const { doc } = createAllTestItems();

‎tests/test-provider/test-provider-context.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ describe('JestTestProviderContext', () => {
2828
const jestRun = context.createTestRun(request, { name: 'test-run' });
2929

3030
expect(jestRun).toBe(mockJestRun);
31-
expect(JestTestRun).toHaveBeenCalledWith('test-run', context, expect.anything());
31+
expect(JestTestRun).toHaveBeenCalledWith('test-run', context, request, expect.anything());
3232
// no vscode run should be created yet
3333
expect(mockController.createTestRun).not.toHaveBeenCalled();
3434

3535
// vscode run will be created through the factory function
36-
const factory = (JestTestRun as jest.Mocked<any>).mock.calls[0][2];
37-
const run = factory('new-test-run');
36+
const factory = (JestTestRun as jest.Mocked<any>).mock.calls[0][3];
37+
const run = factory(request, 'new-test-run');
3838

3939
expect(mockController.createTestRun).toHaveBeenCalledWith(request, 'new-test-run');
4040
expect(run).toBe(mockRun);

‎tests/test-provider/test-provider.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ describe('JestTestProvider', () => {
115115

116116
describe('can discover tests', () => {
117117
it('test mockedJestTestRun', () => {
118-
const jestRun = new JestTestRun('jest-run', {} as any, (() => {}) as any);
118+
const jestRun = new JestTestRun('jest-run', {} as any, {} as any, (() => {}) as any);
119119
expect(jestRun.name).toBe('jest-run');
120120
});
121121
it('should only discover items with canResolveChildren = true', () => {

0 commit comments

Comments
 (0)