chore: check TS in the tests and enable strictNullChecks in there (#…

…2776) - adds a new NPM script called `tsc-check-tests` to run `tsc --noEmit` on all the project files including tests (except E2E tests) - runs this new check inside the tests workflow - enables `strictNullChecks` in the tests, since without that some things cannot work (namely the `Result` type in adaptive crawler, generally speaking, the narrowing of discriminated union types) - adds a lot of non-null assertion operators, the tests were initially written in JS and way too many places work with optional properties as if they were not optional - disables `prefer-destructuring` eslint rule, maybe its just my pet peeve, but I hate it
apify · Dec 19, 2024 · fb84d69 · fb84d69
1 parent 9a6e5f3
commit fb84d69
Show file tree

Hide file tree

Showing 44 changed files with 350 additions and 309 deletions.
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -45,6 +45,8 @@
 					}
 				],
 				"@typescript-eslint/no-empty-interface": "off",
+				"@typescript-eslint/prefer-destructuring": "off",
+				"prefer-destructuring": "off",
 				"no-empty-function": "off",
 				"@typescript-eslint/no-empty-function": "off",
 				"@typescript-eslint/no-explicit-any": "off",

diff --git a/.github/workflows/test-ci.yml b/.github/workflows/test-ci.yml
@@ -70,6 +70,11 @@ jobs:
                 env:
                     YARN_IGNORE_NODE: 1
 
+            -   name: Test TS
+                run: yarn tsc-check-tests
+                env:
+                    YARN_IGNORE_NODE: 1
+
             -   name: Tests
                 run: yarn test
                 env:

diff --git a/package.json b/package.json
@@ -41,6 +41,7 @@
         "test": "vitest run --silent",
         "test:e2e": "node test/e2e/run.mjs",
         "test:full": "cross-env CRAWLEE_DIFFICULT_TESTS=1 vitest run --silent",
+        "tsc-check-tests": "tsc --noEmit --project test/tsconfig.json",
         "coverage": "vitest --coverage",
         "publish:next": "lerna publish from-package --contents dist --dist-tag next --force-publish",
         "release:next": "yarn build && yarn publish:next",

diff --git a/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts b/packages/playwright-crawler/src/internals/adaptive-playwright-crawler.ts
@@ -213,7 +213,7 @@ export class AdaptivePlaywrightCrawler extends PlaywrightCrawler {
     private renderingTypePredictor: NonNullable<AdaptivePlaywrightCrawlerOptions['renderingTypePredictor']>;
     private resultChecker: NonNullable<AdaptivePlaywrightCrawlerOptions['resultChecker']>;
     private resultComparator: NonNullable<AdaptivePlaywrightCrawlerOptions['resultComparator']>;
-    override readonly stats: AdaptivePlaywrightCrawlerStatistics;
+    declare readonly stats: AdaptivePlaywrightCrawlerStatistics;
 
     /**
      * Default {@apilink Router} instance that will be used if we don't specify any {@apilink AdaptivePlaywrightCrawlerOptions.requestHandler|`requestHandler`}.

diff --git a/test/browser-pool/browser-plugins/plugins.test.ts b/test/browser-pool/browser-plugins/plugins.test.ts
@@ -219,7 +219,7 @@ describe('Plugins', () => {
             const page = await browser.newPage();
             const response = await page.goto(`http://127.0.0.1:${(target.address() as AddressInfo).port}`);
 
-            const text = await response.text();
+            const text = await response!.text();
 
             expect(text).toBe('127.0.0.2');
 
@@ -247,7 +247,7 @@ describe('Plugins', () => {
             const page = await browser.newPage();
             const response = await page.goto(`http://127.0.0.1:${(target.address() as AddressInfo).port}`);
 
-            const text = await response.text();
+            const text = await response!.text();
 
             expect(text).toBe('127.0.0.3');
 

diff --git a/test/browser-pool/browser-pool.test.ts b/test/browser-pool/browser-pool.test.ts
@@ -637,11 +637,11 @@ describe.each([
                         ...commonOptions,
                         useFingerprints: true,
                     });
-                    const oldGet = browserPoolConfig.fingerprintGenerator.getFingerprint;
+                    const oldGet = browserPoolConfig.fingerprintGenerator!.getFingerprint;
                     const mock = vitest.fn((options) => {
                         return oldGet.bind(browserPoolConfig.fingerprintGenerator)(options);
                     });
-                    browserPoolConfig.fingerprintGenerator.getFingerprint = mock;
+                    browserPoolConfig.fingerprintGenerator!.getFingerprint = mock;
 
                     const page: Page = await browserPoolConfig.newPage();
                     await page.close();
@@ -674,11 +674,11 @@ describe.each([
                             },
                         },
                     });
-                    const oldGet = browserPoolConfig.fingerprintGenerator.getFingerprint;
+                    const oldGet = browserPoolConfig.fingerprintGenerator!.getFingerprint;
                     const mock = vitest.fn((options) => {
                         return oldGet.bind(browserPoolConfig.fingerprintGenerator)(options);
                     });
-                    browserPoolConfig.fingerprintGenerator.getFingerprint = mock;
+                    browserPoolConfig.fingerprintGenerator!.getFingerprint = mock;
                     const page: Page = await browserPoolConfig.newPageInNewBrowser();
                     await page.close();
                     const [options] = mock.mock.calls[0];

diff --git a/test/core/autoscaling/autoscaled_pool.test.ts b/test/core/autoscaling/autoscaled_pool.test.ts
@@ -25,7 +25,7 @@ describe('AutoscaledPool', () => {
             }
 
             return new Promise((resolve) => {
-                const item = range.shift();
+                const item = range.shift()!;
                 result.push(item);
                 setTimeout(resolve, 5);
             });
@@ -55,7 +55,7 @@ describe('AutoscaledPool', () => {
             }
 
             return new Promise((resolve) => {
-                const item = range.shift();
+                const item = range.shift()!;
                 result.push(item);
                 setTimeout(resolve, 5);
             });
@@ -86,7 +86,7 @@ describe('AutoscaledPool', () => {
             }
 
             return new Promise((resolve) => {
-                const item = range.shift();
+                const item = range.shift()!;
                 result.push(item);
                 setTimeout(resolve, 5);
             });
@@ -418,8 +418,6 @@ describe('AutoscaledPool', () => {
                 if (!aborted) {
                     await pool.abort();
                     aborted = true;
-                } else {
-                    return null;
                 }
             },
             isFinishedFunction: async () => {

diff --git a/test/core/autoscaling/snapshotter.test.ts b/test/core/autoscaling/snapshotter.test.ts
@@ -22,8 +22,8 @@ describe('Snapshotter', () => {
         // mock client data
         const apifyClient = Configuration.getStorageClient();
         const oldStats = apifyClient.stats;
-        apifyClient.stats = {} as never;
-        apifyClient.stats.rateLimitErrors = [0, 0, 0];
+        apifyClient.stats = {} as any;
+        apifyClient.stats!.rateLimitErrors = [0, 0, 0];
 
         const config = new Configuration({ systemInfoIntervalMillis: 100 });
         const snapshotter = new Snapshotter({ config });
@@ -32,7 +32,7 @@ describe('Snapshotter', () => {
         await snapshotter.start();
 
         await sleep(625);
-        apifyClient.stats.rateLimitErrors = [0, 0, 2];
+        apifyClient.stats!.rateLimitErrors = [0, 0, 2];
         await sleep(625);
 
         await snapshotter.stop();
@@ -277,19 +277,19 @@ describe('Snapshotter', () => {
         // mock client data
         const apifyClient = Configuration.getStorageClient();
         const oldStats = apifyClient.stats;
-        apifyClient.stats = {} as never;
-        apifyClient.stats.rateLimitErrors = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
+        apifyClient.stats = {} as any;
+        apifyClient.stats!.rateLimitErrors = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
 
         const snapshotter = new Snapshotter({ maxClientErrors: 1 });
         // @ts-expect-error Calling protected method
         snapshotter._snapshotClient(noop);
-        apifyClient.stats.rateLimitErrors = [1, 1, 1, 0, 0, 0, 0, 0, 0, 0];
+        apifyClient.stats!.rateLimitErrors = [1, 1, 1, 0, 0, 0, 0, 0, 0, 0];
         // @ts-expect-error Calling protected method
         snapshotter._snapshotClient(noop);
-        apifyClient.stats.rateLimitErrors = [10, 5, 2, 0, 0, 0, 0, 0, 0, 0];
+        apifyClient.stats!.rateLimitErrors = [10, 5, 2, 0, 0, 0, 0, 0, 0, 0];
         // @ts-expect-error Calling protected method
         snapshotter._snapshotClient(noop);
-        apifyClient.stats.rateLimitErrors = [100, 24, 4, 2, 0, 0, 0, 0, 0, 0];
+        apifyClient.stats!.rateLimitErrors = [100, 24, 4, 2, 0, 0, 0, 0, 0, 0];
         // @ts-expect-error Calling protected method
         snapshotter._snapshotClient(noop);
 

diff --git a/test/core/browser_launchers/playwright_launcher.test.ts b/test/core/browser_launchers/playwright_launcher.test.ts
@@ -193,7 +193,7 @@ describe('launchPlaywright()', () => {
             });
             const plugin = launcher.createBrowserPlugin();
 
-            expect(plugin!.launchOptions.executablePath).toEqual(target);
+            expect(plugin!.launchOptions!.executablePath).toEqual(target);
         });
 
         test('does not use default when using chrome', () => {
@@ -218,7 +218,7 @@ describe('launchPlaywright()', () => {
             });
             const plugin = launcher.createBrowserPlugin();
 
-            expect(plugin.launchOptions.executablePath).toEqual(newPath);
+            expect(plugin.launchOptions!.executablePath).toEqual(newPath);
         });
 
         test('works without default path', async () => {

diff --git a/test/core/browser_launchers/puppeteer_launcher.test.ts b/test/core/browser_launchers/puppeteer_launcher.test.ts
@@ -17,7 +17,7 @@ import type { Browser, Page } from 'puppeteer';
 
 import { runExampleComServer } from '../../shared/_helper';
 
-let prevEnvHeadless: string;
+let prevEnvHeadless: string | undefined;
 let proxyServer: Server;
 let proxyPort: number;
 const proxyAuth = { scheme: 'Basic', username: 'username', password: 'password' };

diff --git a/test/core/crawlers/adaptive_playwright_crawler.test.ts b/test/core/crawlers/adaptive_playwright_crawler.test.ts
@@ -1,7 +1,7 @@
 import type { Server } from 'http';
 import type { AddressInfo } from 'net';
 
-import { KeyValueStore } from '@crawlee/core';
+import { type Dictionary, KeyValueStore } from '@crawlee/core';
 import type { AdaptivePlaywrightCrawlerOptions } from '@crawlee/playwright';
 import { AdaptivePlaywrightCrawler, RequestList } from '@crawlee/playwright';
 import express from 'express';
@@ -180,7 +180,8 @@ describe('AdaptivePlaywrightCrawler', () => {
 
         const resultChecker: AdaptivePlaywrightCrawlerOptions['resultChecker'] = vi.fn(
             (result) =>
-                result.datasetItems.length > 0 && result.datasetItems.every(({ item }) => item.heading?.length > 0),
+                result.datasetItems.length > 0 &&
+                result.datasetItems.every(({ item }: Dictionary) => item.heading?.length > 0),
         );
 
         const crawler = await makeOneshotCrawler(
@@ -264,7 +265,7 @@ describe('AdaptivePlaywrightCrawler', () => {
 
         await crawler.run();
         const state = await localStorageEmulator.getState();
-        expect(state.value).toEqual({ count: 3 });
+        expect(state!.value).toEqual({ count: 3 });
     });
 
     test('should persist key-value store changes', async () => {
@@ -297,9 +298,9 @@ describe('AdaptivePlaywrightCrawler', () => {
         await crawler.run();
         const store = localStorageEmulator.getKeyValueStore();
 
-        expect((await store.getRecord('1')).value).toEqual({ content: 42 });
-        expect((await store.getRecord('2')).value).toEqual({ content: 42 });
-        expect((await store.getRecord('3')).value).toEqual({ content: 42 });
+        expect((await store.getRecord('1'))!.value).toEqual({ content: 42 });
+        expect((await store.getRecord('2'))!.value).toEqual({ content: 42 });
+        expect((await store.getRecord('3'))!.value).toEqual({ content: 42 });
     });
 
     test('should not allow direct key-value store manipulation', async () => {

diff --git a/test/core/crawlers/basic_crawler.test.ts b/test/core/crawlers/basic_crawler.test.ts
@@ -102,7 +102,7 @@ describe('BasicCrawler', () => {
 
         await basicCrawler.run();
 
-        expect(basicCrawler.autoscaledPool.minConcurrency).toBe(25);
+        expect(basicCrawler.autoscaledPool!.minConcurrency).toBe(25);
         expect(processed).toEqual(sourcesCopy);
         expect(await requestList.isFinished()).toBe(true);
         expect(await requestList.isEmpty()).toBe(true);
@@ -146,12 +146,12 @@ describe('BasicCrawler', () => {
 
         const collectResults = (crawler: BasicCrawler): typeof shorthandOptions | typeof autoscaledPoolOptions => {
             return {
-                minConcurrency: crawler.autoscaledPool.minConcurrency,
-                maxConcurrency: crawler.autoscaledPool.maxConcurrency,
+                minConcurrency: crawler.autoscaledPool!.minConcurrency,
+                maxConcurrency: crawler.autoscaledPool!.maxConcurrency,
                 // eslint-disable-next-line dot-notation -- accessing a private member
-                maxRequestsPerMinute: crawler.autoscaledPool['maxTasksPerMinute'],
+                maxRequestsPerMinute: crawler.autoscaledPool!['maxTasksPerMinute'],
                 // eslint-disable-next-line dot-notation
-                maxTasksPerMinute: crawler.autoscaledPool['maxTasksPerMinute'],
+                maxTasksPerMinute: crawler.autoscaledPool!['maxTasksPerMinute'],
             };
         };
 
@@ -222,7 +222,7 @@ describe('BasicCrawler', () => {
         async (event) => {
             const sources = [...Array(500).keys()].map((index) => ({ url: `https://example.com/${index + 1}` }));
 
-            let persistResolve: (value?: unknown) => void;
+            let persistResolve!: (value?: unknown) => void;
             const persistPromise = new Promise((res) => {
                 persistResolve = res;
             });
@@ -269,7 +269,7 @@ describe('BasicCrawler', () => {
 
             // clean up
             // @ts-expect-error Accessing private method
-            await basicCrawler.autoscaledPool._destroy();
+            await basicCrawler.autoscaledPool!._destroy();
         },
     );
 
@@ -579,7 +579,7 @@ describe('BasicCrawler', () => {
 
         await crawler.run();
 
-        expect(request.retryCount).toBe(0);
+        expect(request!.retryCount).toBe(0);
     });
 
     test('should crash on CriticalError', async () => {
@@ -769,7 +769,7 @@ describe('BasicCrawler', () => {
 
         const isFinishedOrig = vitest.spyOn(requestQueue, 'isFinished');
 
-        requestQueue.fetchNextRequest = async () => Promise.resolve(queue.pop());
+        requestQueue.fetchNextRequest = async () => queue.pop()!;
         requestQueue.isEmpty = async () => Promise.resolve(!queue.length);
 
         setTimeout(() => queue.push(request0), 10);
@@ -818,7 +818,7 @@ describe('BasicCrawler', () => {
 
         const isFinishedOrig = vitest.spyOn(requestQueue, 'isFinished');
 
-        requestQueue.fetchNextRequest = async () => Promise.resolve(queue.pop());
+        requestQueue.fetchNextRequest = async () => Promise.resolve(queue.pop()!);
         requestQueue.isEmpty = async () => Promise.resolve(!queue.length);
 
         setTimeout(() => queue.push(request0), 10);
@@ -1184,8 +1184,8 @@ describe('BasicCrawler', () => {
                     persistStateKey: 'POOL',
                 },
                 requestHandler: async ({ session }) => {
-                    expect(session.constructor.name).toEqual('Session');
-                    expect(session.id).toBeDefined();
+                    expect(session!.constructor.name).toEqual('Session');
+                    expect(session!.id).toBeDefined();
                 },
                 failedRequestHandler: async ({ request }) => {
                     results.push(request);