Skip to content

Commit 733eebf

Browse files
B4nanB4nan
authored
test: run the browser crawler tests serially (#3859)
`browser_crawler.test.ts` is the only test file in the repo using `test.concurrent`, and the only one that gives different results run to run. Locally it failed anywhere between 3 and 10 tests with the set changing every time, which makes it useless for telling whether a change actually broke something — I hit this while trying to work out whether a timeout change had regressed anything, and had to run a control on a clean branch to be sure it hadn't. Running the file serially costs ~28s and makes the outcome deterministic. Worth knowing: locally this does not make the file green — 9 tests fail consistently, identically on a clean v4, because they pass in isolation but not in file order. That pollution is pre-existing and out of scope here; this only removes the variance that was hiding it. If CI is green, those 9 are local-environment-specific and can be looked at separately.
1 parent 8750aaf commit 733eebf

1 file changed

Lines changed: 26 additions & 26 deletions

File tree

‎test/core/crawlers/browser_crawler.test.ts‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ describe('BrowserCrawler', () => {
6161
await run(t);
6262
});
6363

64-
test.concurrent('should work', async () => {
64+
test('should work', async () => {
6565
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
6666

6767
const sources = [
@@ -109,7 +109,7 @@ describe('BrowserCrawler', () => {
109109
});
110110
});
111111

112-
test.concurrent('should teardown browser pool', async () => {
112+
test('should teardown browser pool', async () => {
113113
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
114114

115115
const requestList = await RequestList.open({
@@ -138,7 +138,7 @@ describe('BrowserCrawler', () => {
138138
expect(destroyCalled).toBe(true);
139139
});
140140

141-
test.concurrent('should not tear down a user-supplied browser pool', async () => {
141+
test('should not tear down a user-supplied browser pool', async () => {
142142
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
143143
const externalPool = new BrowserPoolClass({ browserPlugins: [puppeteerPlugin] });
144144

@@ -169,7 +169,7 @@ describe('BrowserCrawler', () => {
169169
}
170170
});
171171

172-
test.concurrent('builds and owns a RemoteBrowserPool from the remoteBrowser option', async () => {
172+
test('builds and owns a RemoteBrowserPool from the remoteBrowser option', async () => {
173173
const crawler = new BrowserCrawlerTest({
174174
remoteBrowser: { endpoint: 'ws://remote:9222', maxOpenBrowsers: 2 },
175175
browserPoolOptions: { browserPlugins: [new PuppeteerPlugin(puppeteer)] },
@@ -182,7 +182,7 @@ describe('BrowserCrawler', () => {
182182
await (crawler.browserPool as RemoteBrowserPool).destroy();
183183
});
184184

185-
test.concurrent('uses browserPool and ignores remoteBrowser when both are set', async () => {
185+
test('uses browserPool and ignores remoteBrowser when both are set', async () => {
186186
const externalPool = new BrowserPoolClass({ browserPlugins: [new PuppeteerPlugin(puppeteer)] });
187187

188188
try {
@@ -198,7 +198,7 @@ describe('BrowserCrawler', () => {
198198
}
199199
});
200200

201-
test.concurrent('should retire session after TimeoutError', async () => {
201+
test('should retire session after TimeoutError', async () => {
202202
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
203203

204204
const requestList = await RequestList.open({
@@ -233,7 +233,7 @@ describe('BrowserCrawler', () => {
233233
expect(markBadCalled).toBe(true);
234234
});
235235

236-
test.concurrent('should evaluate preNavigationHooks', async () => {
236+
test('should evaluate preNavigationHooks', async () => {
237237
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
238238

239239
const requestList = await RequestList.open({
@@ -260,7 +260,7 @@ describe('BrowserCrawler', () => {
260260
expect(hook).toHaveBeenCalled();
261261
});
262262

263-
test.concurrent('should evaluate postNavigationHooks', async () => {
263+
test('should evaluate postNavigationHooks', async () => {
264264
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
265265

266266
const requestList = await RequestList.open({
@@ -287,7 +287,7 @@ describe('BrowserCrawler', () => {
287287
expect(hook).toHaveBeenCalled();
288288
});
289289

290-
test.concurrent('postNavigationHooks can override response, observed downstream', async () => {
290+
test('postNavigationHooks can override response, observed downstream', async () => {
291291
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
292292

293293
const requestList = await RequestList.open({
@@ -327,7 +327,7 @@ describe('BrowserCrawler', () => {
327327
expect(observed.fromHandler).toBe(fakeStatus);
328328
});
329329

330-
test.concurrent('errorHandler has open page', async () => {
330+
test('errorHandler has open page', async () => {
331331
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
332332

333333
const requestList = await RequestList.open({
@@ -356,7 +356,7 @@ describe('BrowserCrawler', () => {
356356
expect(result[0]).toBe(serverAddress);
357357
});
358358

359-
test.concurrent('should correctly track request.state', async () => {
359+
test('should correctly track request.state', async () => {
360360
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
361361

362362
const sources = [{ url: `${serverAddress}/?q=1` }];
@@ -401,7 +401,7 @@ describe('BrowserCrawler', () => {
401401
]);
402402
});
403403

404-
test.concurrent('should allow modifying gotoOptions by pre navigation hooks', async () => {
404+
test('should allow modifying gotoOptions by pre navigation hooks', async () => {
405405
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
406406

407407
const requestList = await RequestList.open({
@@ -436,7 +436,7 @@ describe('BrowserCrawler', () => {
436436
expect(optionsGoto!.timeout).toEqual(60000);
437437
});
438438

439-
test.concurrent('should ignore errors in Page.close()', async () => {
439+
test('should ignore errors in Page.close()', async () => {
440440
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
441441

442442
for (let i = 0; i < 2; i++) {
@@ -469,7 +469,7 @@ describe('BrowserCrawler', () => {
469469
}
470470
});
471471

472-
test.concurrent('should respect the requestHandlerTimeoutSecs option', async () => {
472+
test('should respect the requestHandlerTimeoutSecs option', async () => {
473473
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
474474

475475
const requestList = await RequestList.open({
@@ -499,7 +499,7 @@ describe('BrowserCrawler', () => {
499499
expect(callSpy).toBeCalledWith('good');
500500
});
501501

502-
test.concurrent('should not throw without SessionPool', async () => {
502+
test('should not throw without SessionPool', async () => {
503503
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
504504

505505
const requestList = await RequestList.open({
@@ -517,7 +517,7 @@ describe('BrowserCrawler', () => {
517517
expect(browserCrawler).toBeDefined();
518518
});
519519

520-
test.concurrent('should correctly set session pool options', async () => {
520+
test('should correctly set session pool options', async () => {
521521
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
522522

523523
const requestList = await RequestList.open({
@@ -601,7 +601,7 @@ describe('BrowserCrawler', () => {
601601
});
602602
});
603603

604-
test.concurrent('should throw on "blocked" status codes', async () => {
604+
test('should throw on "blocked" status codes', async () => {
605605
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
606606

607607
const baseUrl = 'https://example.com/';
@@ -646,7 +646,7 @@ describe('BrowserCrawler', () => {
646646
expect(called).toBe(false);
647647
});
648648

649-
test.concurrent('retryOnBlocked should retry on Cloudflare challenge', async () => {
649+
test('retryOnBlocked should retry on Cloudflare challenge', async () => {
650650
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
651651

652652
const urls = [new URL('/special/cloudflareBlocking', serverAddress).href];
@@ -676,7 +676,7 @@ describe('BrowserCrawler', () => {
676676
expect(processed).toBe(false);
677677
});
678678

679-
test.concurrent('retryOnBlocked throws on "blocked" status codes', async () => {
679+
test('retryOnBlocked throws on "blocked" status codes', async () => {
680680
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
681681

682682
const baseUrl = 'https://example.com/';
@@ -718,7 +718,7 @@ describe('BrowserCrawler', () => {
718718
expect(processed).toBe(false);
719719
});
720720

721-
test.concurrent('should throw on "blocked" status codes (retire session)', async () => {
721+
test('should throw on "blocked" status codes (retire session)', async () => {
722722
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
723723

724724
const baseUrl = 'https://example.com/';
@@ -763,7 +763,7 @@ describe('BrowserCrawler', () => {
763763
expect(called).toBe(false);
764764
});
765765

766-
test.concurrent('should retire browser with session', async () => {
766+
test('should retire browser with session', async () => {
767767
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
768768

769769
const requestList = await RequestList.open({
@@ -790,7 +790,7 @@ describe('BrowserCrawler', () => {
790790
expect(retiredBrowserCount).toBeGreaterThan(0);
791791
});
792792

793-
test.concurrent('should increment session usage correctly', async () => {
793+
test('should increment session usage correctly', async () => {
794794
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
795795

796796
const sessionUsageHistory: number[] = [];
@@ -819,7 +819,7 @@ describe('BrowserCrawler', () => {
819819
expect(sessionUsageHistory).toEqual([0, 1, 2, 3, 4, 5]);
820820
});
821821

822-
test.concurrent('should allow using fingerprints from browser pool', async () => {
822+
test('should allow using fingerprints from browser pool', async () => {
823823
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
824824

825825
const pool = new BrowserPoolClass({
@@ -902,7 +902,7 @@ describe('BrowserCrawler', () => {
902902
delete process.env[ENV_VARS.PROXY_PASSWORD];
903903
});
904904

905-
test.concurrent('proxy rotation on error works as expected', async () => {
905+
test('proxy rotation on error works as expected', async () => {
906906
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
907907

908908
const requestList = await RequestList.open({
@@ -949,7 +949,7 @@ describe('BrowserCrawler', () => {
949949
expect(requestHandler).toHaveBeenCalledTimes(4);
950950
});
951951

952-
test.concurrent('proxy rotation on error respects maxRequestRetries, calls failedRequestHandler', async () => {
952+
test('proxy rotation on error respects maxRequestRetries, calls failedRequestHandler', async () => {
953953
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
954954

955955
const requestList = await RequestList.open({
@@ -1001,7 +1001,7 @@ describe('BrowserCrawler', () => {
10011001
expect(numberOfRotations).toBe(4 * 5);
10021002
});
10031003

1004-
test.concurrent('proxy rotation logs the original proxy error', async () => {
1004+
test('proxy rotation logs the original proxy error', async () => {
10051005
const puppeteerPlugin = new PuppeteerPlugin(puppeteer);
10061006

10071007
const requestList = await RequestList.open({

0 commit comments

Comments
 (0)