Skip to content

Commit c8868f4

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

File tree

5 files changed

+56
-42
lines changed

5 files changed

+56
-42
lines changed

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

Lines changed: 21 additions & 7 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,46 @@ 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, this whole method is sync
159160
if (!isSelectivityEnabledForBrowser || this._opts?.shouldDisableSelectivity) {
160161
return this._runTestFn(test, browserId);
161162
}
162163

163164
this._processingTestPromises.push(
164-
(async (): Promise<void> => {
165+
(async (): Promise<[Test, string] | null> => {
165166
const shouldDisableBrowserSelectivity = await this._shouldDisableSelectivityForBrowser(browserId);
166167

167168
if (shouldDisableBrowserSelectivity) {
168-
return this._runTestFn(test, browserId);
169+
return [test, browserId];
169170
}
170171

171172
const shouldDisableTest = await shouldDisableTestBySelectivity(browserConfig, test);
172173

173174
if (!shouldDisableTest) {
174-
this._runTestFn(test, browserId);
175+
return [test, browserId];
175176
}
177+
178+
return null;
176179
})(),
177180
);
178181
}
179182

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

‎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: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ describe("SelectivityRunner", () => {
115115
it("should run test immediately if selectivity is disabled for browser", () => {
116116
browserConfigMock.selectivity.enabled = false;
117117

118-
runner.runIfNecessary(testMock, "chrome");
118+
runner.startTestCheckToRun(testMock, "chrome");
119119

120120
assert.calledOnce(runTestFnMock);
121121
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -129,7 +129,7 @@ describe("SelectivityRunner", () => {
129129
{ shouldDisableSelectivity: true },
130130
);
131131

132-
runnerWithDisabledSelectivity.runIfNecessary(testMock, "chrome");
132+
runnerWithDisabledSelectivity.startTestCheckToRun(testMock, "chrome");
133133

134134
assert.calledOnce(runTestFnMock);
135135
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -144,7 +144,7 @@ describe("SelectivityRunner", () => {
144144
{ shouldDisableSelectivity: true },
145145
);
146146

147-
runnerWithDisabledSelectivity.runIfNecessary(testMock, "chrome");
147+
runnerWithDisabledSelectivity.startTestCheckToRun(testMock, "chrome");
148148

149149
assert.calledOnce(runTestFnMock);
150150
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -157,9 +157,9 @@ describe("SelectivityRunner", () => {
157157
testDepsReaderMock.getFor.resolves(testDeps);
158158
hashReaderMock.getTestChangedDeps.resolves({ css: [], js: ["src/app.js"], modules: [] });
159159

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

162-
await runner.waitForTestsToRun();
162+
await runner.runNecessaryTests();
163163

164164
assert.calledOnce(runTestFnMock);
165165
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -169,9 +169,9 @@ describe("SelectivityRunner", () => {
169169
browserConfigMock.selectivity.disableSelectivityPatterns = ["src/**/*.js"];
170170
hashReaderMock.patternHasChanged.resolves(true);
171171

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

174-
await runner.waitForTestsToRun();
174+
await runner.runNecessaryTests();
175175

176176
assert.calledOnce(runTestFnMock);
177177
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -188,9 +188,9 @@ describe("SelectivityRunner", () => {
188188
const error = new Error("Pattern check failed");
189189
hashReaderMock.patternHasChanged.rejects(error);
190190

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

193-
await runner.waitForTestsToRun();
193+
await runner.runNecessaryTests();
194194

195195
assert.calledOnce(runTestFnMock);
196196
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -210,9 +210,9 @@ describe("SelectivityRunner", () => {
210210
testDepsReaderMock.getFor.resolves(testDeps);
211211
hashReaderMock.getTestChangedDeps.resolves(changedDeps);
212212

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

215-
await runner.waitForTestsToRun();
215+
await runner.runNecessaryTests();
216216

217217
assert.calledOnce(runTestFnMock);
218218
assert.calledWith(runTestFnMock, testMock, "chrome");
@@ -234,9 +234,9 @@ describe("SelectivityRunner", () => {
234234
testDepsReaderMock.getFor.resolves(testDeps);
235235
hashReaderMock.getTestChangedDeps.resolves(null); // No changes
236236

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

239-
await runner.waitForTestsToRun();
239+
await runner.runNecessaryTests();
240240

241241
assert.notCalled(runTestFnMock);
242242
assert.calledWith(getTestDependenciesReaderStub, "/test/path", "none");
@@ -255,9 +255,9 @@ describe("SelectivityRunner", () => {
255255
const testDeps = { css: ["src/styles.css"], js: [], modules: [] };
256256
testDepsReaderMock.getFor.resolves(testDeps);
257257

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

260-
await runner.waitForTestsToRun();
260+
await runner.runNecessaryTests();
261261

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

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

285-
await runner.waitForTestsToRun();
285+
await runner.runNecessaryTests();
286286

287287
assert.notCalled(runTestFnMock);
288288
assert.calledOnce(hashReaderMock.patternHasChanged); // Should be cached for same browser
@@ -315,27 +315,26 @@ describe("SelectivityRunner", () => {
315315
testDepsReaderMock.getFor.resolves(testDeps);
316316
hashReaderMock.getTestChangedDeps.resolves(null);
317317

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

321-
await runner.waitForTestsToRun();
321+
await runner.runNecessaryTests();
322322

323323
assert.notCalled(runTestFnMock);
324324
});
325325
});
326326

327-
describe("waitForTestsToRun", () => {
327+
describe("runNecessaryTests", () => {
328328
let runner: SelectivityRunner;
329329

330330
beforeEach(() => {
331331
runner = new SelectivityRunnerClass(mainRunnerMock as any, configMock as any, runTestFnMock);
332332
});
333333

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

337-
assert.isArray(result);
338-
assert.lengthOf(result, 0);
337+
assert.notCalled(runTestFnMock);
339338
});
340339

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

362-
runner.runIfNecessary(testMock, "chrome");
363-
runner.runIfNecessary(testMock, "chrome");
361+
runner.startTestCheckToRun(testMock, "chrome");
362+
runner.startTestCheckToRun(testMock, "chrome");
364363

365-
const result = await runner.waitForTestsToRun();
364+
await runner.runNecessaryTests();
366365

367-
assert.isArray(result);
368-
assert.lengthOf(result, 2);
366+
assert.calledTwice(runTestFnMock);
369367
});
370368
});
371369

‎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)