diff --git a/backend/eslint.config.mjs b/backend/eslint.config.mjs index 51cc30f..07af853 100644 --- a/backend/eslint.config.mjs +++ b/backend/eslint.config.mjs @@ -50,9 +50,6 @@ export default tseslint.config( ), { - // `tests/` is deliberately out of scope for now: tsconfig.json only includes - // `src`, so type-aware linting has no program for the test files, and - // widening it is a separate change with its own violation count. files: ['src/**/*.ts'], languageOptions: { globals: globals.node, @@ -71,5 +68,73 @@ export default tseslint.config( { checksVoidReturn: { attributes: false } }, ], }, + }, + + { + // The test suites, in scope since #298. They had never been linted at all: + // this config said tests were out of scope because tsconfig.json includes + // only `src`, and that stayed true for long enough that two defects lived + // here undetected — a unit test that opened a real TLS connection to Gmail + // on every run, and integration tests that mocked the shared pg pool and + // made a suite unrunnable. Neither is something lint would necessarily have + // caught, but neither was ever looked at. + // + // `project` rather than `projectService`, for the reason the frontend's + // equivalent block records: the service resolves each file to the nearest + // tsconfig.json, which for tests/ is the one that excludes them, and every + // file then errors as not part of a project. + files: ['tests/**/*.ts'], + languageOptions: { + globals: { ...globals.node, ...globals.jest }, + parserOptions: { + project: ['./tsconfig.test.json'], + tsconfigRootDir: import.meta.dirname, + }, + }, + rules: { + // The same rule src is held to, and it matters at least as much here. + // An unawaited promise in a test does not fail the test — it passes, + // having asserted nothing, and the failure surfaces later as a suite that + // will not exit. + '@typescript-eslint/no-floating-promises': 'error', + '@typescript-eslint/no-misused-promises': [ + 'error', + { checksVoidReturn: { attributes: false } }, + ], + + // Everything below is switched off for tests rather than left as a + // warning, on #60's argument: bringing these files in scope produced 77 + // warnings, of which 60 were rules that cannot be true in a test. A rule + // that cannot be true here is noise, and noise hides the rules that can. + // What is left is signal — unused variables, useless escapes, a regex + // worth a second look. + + // 41 of the 77. Test credentials are the entire point of a test, and this + // project's own rule is that they must live only in test paths — which is + // here. Flagging them where they belong trains a reader to skip the rule + // where they do not. + 'sonarjs/no-hardcoded-passwords': 'off', + + // Stub servers and fixtures: `http://127.0.0.1:`. There is no + // transport to secure between a test and a socket it opened itself. + 'sonarjs/no-clear-text-protocols': 'off', + + // 203.0.113.5 is TEST-NET-3, reserved by RFC 5737 for exactly this. A + // documentation address is the correct thing to hardcode. + 'sonarjs/no-hardcoded-ip': 'off', + + // `os.tmpdir()`, via mkdtemp, which is how these suites get a scratch + // uploads directory they can delete afterwards. + 'sonarjs/publicly-writable-directories': 'off', + + // Math.random for a run id. Nothing here is a secret; it only has to not + // collide with a parallel worker. + 'sonarjs/pseudo-random': 'off', + + // Sorting two string arrays to compare them is how several guards assert + // set equality. The locale-aware comparator the rule wants would change + // nothing except the reading. + 'sonarjs/no-alphabetical-sort': 'off', + }, } ); diff --git a/backend/package.json b/backend/package.json index 884b8c1..c55db81 100755 --- a/backend/package.json +++ b/backend/package.json @@ -8,7 +8,7 @@ }, "scripts": { "build": "tsc", - "lint": "eslint src scripts", + "lint": "eslint src scripts tests", "start": "node dist/server.js", "dev": "tsx watch src/server.ts", "test": "npm run test:unit", diff --git a/backend/tests/integration/setup/testDb.ts b/backend/tests/integration/setup/testDb.ts index 6f9da48..98018a5 100755 --- a/backend/tests/integration/setup/testDb.ts +++ b/backend/tests/integration/setup/testDb.ts @@ -123,8 +123,16 @@ export async function resetDb(): Promise { // Same reasoning, same failure, for the intake settings (#227): a ceiling of // 1 left behind by the ceiling suite makes every later submission refuse with // a 503, in files that never mention a ceiling. + // + // ESCAPE, and not a backslash, because the backslash that used to be here did + // nothing. In a JavaScript string 'email\_%' is 'email_%', and `_` in SQL LIKE + // matches any single character — so the pattern meant "email plus any one + // character", not "email_". It happened to delete the right rows only because + // no other key begins with those letters followed by something else; a + // setting called emailing_enabled would have been swept away silently. Found + // by no-useless-escape the first time lint was pointed at tests/ (#298). await testPool.query( - `DELETE FROM admin_settings WHERE key LIKE 'email\_%' OR key LIKE 'intake\_%'` + `DELETE FROM admin_settings WHERE key LIKE 'email!_%' ESCAPE '!' OR key LIKE 'intake!_%' ESCAPE '!'` ); } diff --git a/backend/tests/integration/uploadValidation.integration.test.ts b/backend/tests/integration/uploadValidation.integration.test.ts index 5f57a80..1f38e80 100644 --- a/backend/tests/integration/uploadValidation.integration.test.ts +++ b/backend/tests/integration/uploadValidation.integration.test.ts @@ -151,8 +151,6 @@ describe('what it refuses', () => { }); it('leaves nothing on the volume when it refuses the content', async () => { - const before: string[] = []; - await createItem().attach('images', HTML_BYTES, { filename: 'photo.jpg', contentType: 'image/jpeg' @@ -164,8 +162,6 @@ describe('what it refuses', () => { // A rejection must take the whole request with it. Accepting the good file // from a refused upload would leave a file behind that nothing references. it('discards the valid files from a request that also carried an invalid one', async () => { - const before: string[] = []; - const res = await createItem() .attach('images', REAL_PNG, { filename: 'good.png', contentType: 'image/png' }) .attach('images', HTML_BYTES, { filename: 'bad.jpg', contentType: 'image/jpeg' }); @@ -209,8 +205,6 @@ describe('the uploads directory', () => { */ describe('what it leaves on disk', () => { it('removes the upload when the request is refused for its other fields', async () => { - const before: string[] = []; - const res = await request(app) .post('/api/admin/items') .field('name', 'Photographed thing') @@ -226,8 +220,6 @@ describe('what it leaves on disk', () => { }); it('removes the upload when the tags field is refused', async () => { - const before: string[] = []; - const res = await request(app) .post('/api/admin/items') .field('name', 'Photographed thing') @@ -243,8 +235,6 @@ describe('what it leaves on disk', () => { // The accepted case must not be swept up by the same cleanup: these files are // the ones the item now points at. it('keeps the upload when the request succeeds', async () => { - const before: string[] = []; - const res = await createItem().attach('images', REAL_PNG, { filename: 'photo.png', contentType: 'image/png' diff --git a/backend/tsconfig.test.json b/backend/tsconfig.test.json new file mode 100644 index 0000000..812465f --- /dev/null +++ b/backend/tsconfig.test.json @@ -0,0 +1,9 @@ +{ + "//": "Exists so ESLint has a TypeScript program covering tests/. tsconfig.json includes only `src`, which is why tests were out of scope for linting until #298 — type-aware rules had nothing to resolve them against. Separate from tsconfig.json rather than widening its `include`, because that one drives `npm run build` and emits to dist/: pulling the suite into the build output would ship the tests. Mirrors frontend/tsconfig.test.json, which exists for the same reason (#137). The files are already type-checked at run time by ts-jest; this adds nothing to that, only to what the linter can see.", + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": true, + "rootDir": "." + }, + "include": ["tests", "src"] +}