Skip to content

Commit 3e19fae

Browse files
KuznetsovRomanKuznetsovRoman
committed
fix(selectivity): add tests to run synchronously
1 parent 5f56305 commit 3e19fae

File tree

5 files changed

+67
-46
lines changed

5 files changed

+67
-46
lines changed

‎src/browser/cdp/selectivity/runner.ts‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export class SelectivityRunner {
117117
private readonly _runTestFn: (test: Test, browserId: string) => void;
118118
private readonly _opts?: SelectivityRunnerOptions;
119119
private readonly _browserSelectivityDisabledCache: Record<string, void | Promise<boolean>> = {};
120-
private readonly _processingTestPromises: Promise<void>[] = [];
120+
private readonly _processingTestPromises: Promise<[Test, string] | null>[] = [];
121121

122122
static create(...args: ConstructorParameters<typeof this>): SelectivityRunner {
123123
return new this(...args);
@@ -152,32 +152,50 @@ export class SelectivityRunner {
152152
return this._browserSelectivityDisabledCache[browserId] as Promise<boolean>;
153153
}
154154

155-
runIfNecessary(test: Test, browserId: string): void {
155+
startTestCheckToRun(test: Test, browserId: string): void {
156156
const browserConfig = this._config.forBrowser(browserId);
157157
const isSelectivityEnabledForBrowser = browserConfig.selectivity.enabled;
158158

159+
// If selectivity is disabled for browser
159160
if (!isSelectivityEnabledForBrowser || this._opts?.shouldDisableSelectivity) {
160-
return this._runTestFn(test, browserId);
161+
this._processingTestPromises.push(Promise.resolve([test, browserId]));
162+
return;
161163
}
162164

163165
this._processingTestPromises.push(
164-
(async (): Promise<void> => {
166+
(async (): Promise<[Test, string] | null> => {
165167
const shouldDisableBrowserSelectivity = await this._shouldDisableSelectivityForBrowser(browserId);
166168

167169
if (shouldDisableBrowserSelectivity) {
168-
return this._runTestFn(test, browserId);
170+
return [test, browserId];
169171
}
170172

171173
const shouldDisableTest = await shouldDisableTestBySelectivity(browserConfig, test);
172174

173175
if (!shouldDisableTest) {
174-
this._runTestFn(test, browserId);
176+
return [test, browserId];
175177
}
178+
179+
return null;
176180
})(),
177181
);
178182
}
179183

180-
waitForTestsToRun(): Promise<void[]> {
181-
return Promise.all(this._processingTestPromises);
184+
async runNecessaryTests(): Promise<void> {
185+
const testsToRun = await Promise.all(this._processingTestPromises);
186+
187+
for (const testToRun of testsToRun) {
188+
if (!testToRun) {
189+
continue;
190+
}
191+
192+
const [test, browserId] = testToRun;
193+
194+
// All tests need to be started synchronously
195+
this._runTestFn(test, browserId);
196+
}
197+
198+
// Free used memory
199+
this._processingTestPromises.length = 0;
182200
}
183201
}

‎src/runner/index.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,11 @@ export class MainRunner extends RunnableEmitter {
134134
shouldDisableSelectivity: opts?.shouldDisableSelectivity,
135135
});
136136

137-
testCollection.eachTestAcrossBrowsers((test, browserId) => selectivityRunner.runIfNecessary(test, browserId));
137+
testCollection.eachTestAcrossBrowsers((test, browserId) =>
138+
selectivityRunner.startTestCheckToRun(test, browserId),
139+
);
138140

139-
await selectivityRunner.waitForTestsToRun();
141+
await selectivityRunner.runNecessaryTests();
140142

141143
this.activeBrowserRunners.forEach(runner => this.running.add(this._waitBrowserRunnerTestsCompletion(runner)));
142144

‎test/src/browser/cdp/selectivity/runner.ts‎

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -112,30 +112,32 @@ describe("SelectivityRunner", () => {
112112
runner = new SelectivityRunnerClass(mainRunnerMock as any, configMock as any, runTestFnMock);
113113
});
114114

115-
it("should run test immediately if selectivity is disabled for browser", () => {
115+
it("should run test if selectivity is disabled for browser", async () => {
116116
browserConfigMock.selectivity.enabled = false;
117117

118-
runner.runIfNecessary(testMock, "chrome");
118+
runner.startTestCheckToRun(testMock, "chrome");
119+
await runner.runNecessaryTests();
119120

120121
assert.calledOnce(runTestFnMock);
121122
assert.calledWith(runTestFnMock, testMock, "chrome");
122123
});
123124

124-
it("should run test immediately if shouldDisableSelectivity option is true", () => {
125+
it("should run test if shouldDisableSelectivity option is true", async () => {
125126
const runnerWithDisabledSelectivity = new SelectivityRunnerClass(
126127
mainRunnerMock as any,
127128
configMock as any,
128129
runTestFnMock,
129130
{ shouldDisableSelectivity: true },
130131
);
131132

132-
runnerWithDisabledSelectivity.runIfNecessary(testMock, "chrome");
133+
runnerWithDisabledSelectivity.startTestCheckToRun(testMock, "chrome");
134+
await runnerWithDisabledSelectivity.runNecessaryTests();
133135

134136
assert.calledOnce(runTestFnMock);
135137
assert.calledWith(runTestFnMock, testMock, "chrome");
136138
});
137139

138-
it("should run test immediately if both browser selectivity is disabled and shouldDisableSelectivity is true", () => {
140+
it("should run test if both browser selectivity is disabled and shouldDisableSelectivity is true", async () => {
139141
browserConfigMock.selectivity.enabled = false;
140142
const runnerWithDisabledSelectivity = new SelectivityRunnerClass(
141143
mainRunnerMock as any,
@@ -144,7 +146,8 @@ describe("SelectivityRunner", () => {
144146
{ shouldDisableSelectivity: true },
145147
);
146148

147-
runnerWithDisabledSelectivity.runIfNecessary(testMock, "chrome");
149+
runnerWithDisabledSelectivity.startTestCheckToRun(testMock, "chrome");
150+
await runnerWithDisabledSelectivity.runNecessaryTests();
148151

149152
assert.calledOnce(runTestFnMock);
150153
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -157,9 +160,9 @@ describe("SelectivityRunner", () => {
157160
testDepsReaderMock.getFor.resolves(testDeps);
158161
hashReaderMock.getTestChangedDeps.resolves({ css: [], js: ["src/app.js"], modules: [] });
159162

160-
runner.runIfNecessary(testMock, "chrome");
163+
runner.startTestCheckToRun(testMock, "chrome");
161164

162-
await runner.waitForTestsToRun();
165+
await runner.runNecessaryTests();
163166

164167
assert.calledOnce(runTestFnMock);
165168
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -169,9 +172,9 @@ describe("SelectivityRunner", () => {
169172
browserConfigMock.selectivity.disableSelectivityPatterns = ["src/**/*.js"];
170173
hashReaderMock.patternHasChanged.resolves(true);
171174

172-
runner.runIfNecessary(testMock, "chrome");
175+
runner.startTestCheckToRun(testMock, "chrome");
173176

174-
await runner.waitForTestsToRun();
177+
await runner.runNecessaryTests();
175178

176179
assert.calledOnce(runTestFnMock);
177180
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -188,9 +191,9 @@ describe("SelectivityRunner", () => {
188191
const error = new Error("Pattern check failed");
189192
hashReaderMock.patternHasChanged.rejects(error);
190193

191-
runner.runIfNecessary(testMock, "chrome");
194+
runner.startTestCheckToRun(testMock, "chrome");
192195

193-
await runner.waitForTestsToRun();
196+
await runner.runNecessaryTests();
194197

195198
assert.calledOnce(runTestFnMock);
196199
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -210,9 +213,9 @@ describe("SelectivityRunner", () => {
210213
testDepsReaderMock.getFor.resolves(testDeps);
211214
hashReaderMock.getTestChangedDeps.resolves(changedDeps);
212215

213-
runner.runIfNecessary(testMock, "chrome");
216+
runner.startTestCheckToRun(testMock, "chrome");
214217

215-
await runner.waitForTestsToRun();
218+
await runner.runNecessaryTests();
216219

217220
assert.calledOnce(runTestFnMock);
218221
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -234,9 +237,9 @@ describe("SelectivityRunner", () => {
234237
testDepsReaderMock.getFor.resolves(testDeps);
235238
hashReaderMock.getTestChangedDeps.resolves(null); // No changes
236239

237-
runner.runIfNecessary(testMock, "chrome");
240+
runner.startTestCheckToRun(testMock, "chrome");
238241

239-
await runner.waitForTestsToRun();
242+
await runner.runNecessaryTests();
240243

241244
assert.notCalled(runTestFnMock);
242245
assert.calledWith(getTestDependenciesReaderStub, "/test/path", "none");
@@ -255,9 +258,9 @@ describe("SelectivityRunner", () => {
255258
const testDeps = { css: ["src/styles.css"], js: [], modules: [] };
256259
testDepsReaderMock.getFor.resolves(testDeps);
257260

258-
runner.runIfNecessary(testMock, "chrome");
261+
runner.startTestCheckToRun(testMock, "chrome");
259262

260-
await runner.waitForTestsToRun();
263+
await runner.runNecessaryTests();
261264

262265
assert.calledOnce(runTestFnMock); // Should run because no JS deps means it's new
263266
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -279,10 +282,10 @@ describe("SelectivityRunner", () => {
279282
const test1 = { ...testMock, id: "test-1" } as Test;
280283
const test2 = { ...testMock, id: "test-2" } as Test;
281284

282-
runner.runIfNecessary(test1, "chrome");
283-
runner.runIfNecessary(test2, "chrome");
285+
runner.startTestCheckToRun(test1, "chrome");
286+
runner.startTestCheckToRun(test2, "chrome");
284287

285-
await runner.waitForTestsToRun();
288+
await runner.runNecessaryTests();
286289

287290
assert.notCalled(runTestFnMock);
288291
assert.calledOnce(hashReaderMock.patternHasChanged); // Should be cached for same browser
@@ -315,27 +318,26 @@ describe("SelectivityRunner", () => {
315318
testDepsReaderMock.getFor.resolves(testDeps);
316319
hashReaderMock.getTestChangedDeps.resolves(null);
317320

318-
runner.runIfNecessary(testMock, "chrome");
319-
runner.runIfNecessary(testMock, "firefox");
321+
runner.startTestCheckToRun(testMock, "chrome");
322+
runner.startTestCheckToRun(testMock, "firefox");
320323

321-
await runner.waitForTestsToRun();
324+
await runner.runNecessaryTests();
322325

323326
assert.notCalled(runTestFnMock);
324327
});
325328
});
326329

327-
describe("waitForTestsToRun", () => {
330+
describe("runNecessaryTests", () => {
328331
let runner: SelectivityRunner;
329332

330333
beforeEach(() => {
331334
runner = new SelectivityRunnerClass(mainRunnerMock as any, configMock as any, runTestFnMock);
332335
});
333336

334337
it("should resolve immediately if no tests are processing", async () => {
335-
const result = await runner.waitForTestsToRun();
338+
await runner.runNecessaryTests();
336339

337-
assert.isArray(result);
338-
assert.lengthOf(result, 0);
340+
assert.notCalled(runTestFnMock);
339341
});
340342

341343
it("should wait for all processing tests to complete", async () => {
@@ -359,13 +361,12 @@ describe("SelectivityRunner", () => {
359361
testDepsReaderMock.getFor.resolves(testDeps);
360362
hashReaderMock.getTestChangedDeps.resolves({ css: [], js: ["src/app.js"], modules: [] });
361363

362-
runner.runIfNecessary(testMock, "chrome");
363-
runner.runIfNecessary(testMock, "chrome");
364+
runner.startTestCheckToRun(testMock, "chrome");
365+
runner.startTestCheckToRun(testMock, "chrome");
364366

365-
const result = await runner.waitForTestsToRun();
367+
await runner.runNecessaryTests();
366368

367-
assert.isArray(result);
368-
assert.lengthOf(result, 2);
369+
assert.calledTwice(runTestFnMock);
369370
});
370371
});
371372

‎test/src/browser/cdp/selectivity/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ describe("CDP/Selectivity/Utils", () => {
180180

181181
beforeEach(() => {
182182
consumerMock = { originalPositionFor: sandbox.stub() };
183-
SourceMapConsumerStub.resolves(consumerMock);
183+
SourceMapConsumerStub.returns(consumerMock);
184184
});
185185

186186
it("should extract source files from coverage offsets", async () => {

‎test/src/runner/index.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ describe("NodejsEnvRunner", () => {
7272
BrowserPool = { create: sinon.stub().returns({ cancel: sandbox.spy() }) };
7373

7474
const createSelectivityRunnerStub = (mainRunner, config, runTestFn) => ({
75-
runIfNecessary: (...args) => runTestFn(...args),
76-
waitForTestsToRun: sinon.stub().resolves(),
75+
startTestCheckToRun: (...args) => runTestFn(...args),
76+
runNecessaryTests: sinon.stub().resolves(),
7777
});
7878

7979
SelectivityRunner = { create: sinon.stub().callsFake(createSelectivityRunnerStub) };

0 commit comments

Comments
 (0)