From ecd304fba00e2383357bb7212402c45371459720 Mon Sep 17 00:00:00 2001 From: Anders Hassis Date: Sun, 8 Mar 2026 10:43:44 +0100 Subject: [PATCH 1/4] feat: add test coverage tooling with @vitest/coverage-v8 Configure v8 coverage provider in both core and cli packages with text, lcov, and json-summary reporters. Add test:coverage scripts at package and root level. Add coverage/ to .gitignore. --- .gitignore | 1 + package-lock.json | 137 +++++++++++++++++++++++++++++++++ package.json | 1 + packages/cli/package.json | 2 + packages/cli/vitest.config.ts | 7 ++ packages/core/package.json | 2 + packages/core/vitest.config.ts | 7 ++ 7 files changed, 157 insertions(+) diff --git a/.gitignore b/.gitignore index d42fa4f..52a977b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ dist/ +coverage/ node_modules/ .env .env.* diff --git a/package-lock.json b/package-lock.json index 3412fcb..2b1c1f0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -541,6 +541,16 @@ "node": ">=6.9.0" } }, + "node_modules/@bcoe/v8-coverage": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/@bcoe/v8-coverage/-/v8-coverage-1.0.2.tgz", + "integrity": "sha512-6zABk/ECA/QYSCQ1NGiVwwbQerUCZ+TQbp64Q3AgmfNvurHH0j8TtXa1qbShXA6qqkpAj4V5W8pP6mLe1mcMqA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + } + }, "node_modules/@capsizecss/unpack": { "version": "4.0.0", "license": "MIT", @@ -2544,6 +2554,37 @@ "vite": "^4.2.0 || ^5.0.0 || ^6.0.0 || ^7.0.0" } }, + "node_modules/@vitest/coverage-v8": { + "version": "4.0.18", + "resolved": "https://registry.npmjs.org/@vitest/coverage-v8/-/coverage-v8-4.0.18.tgz", + "integrity": "sha512-7i+N2i0+ME+2JFZhfuz7Tg/FqKtilHjGyGvoHYQ6iLV0zahbsJ9sljC9OcFcPDbhYKCet+sG8SsVqlyGvPflZg==", + "dev": true, + "license": "MIT", + "dependencies": { + "@bcoe/v8-coverage": "^1.0.2", + "@vitest/utils": "4.0.18", + "ast-v8-to-istanbul": "^0.3.10", + "istanbul-lib-coverage": "^3.2.2", + "istanbul-lib-report": "^3.0.1", + "istanbul-reports": "^3.2.0", + "magicast": "^0.5.1", + "obug": "^2.1.1", + "std-env": "^3.10.0", + "tinyrainbow": "^3.0.3" + }, + "funding": { + "url": "https://opencollective.com/vitest" + }, + "peerDependencies": { + "@vitest/browser": "4.0.18", + "vitest": "4.0.18" + }, + "peerDependenciesMeta": { + "@vitest/browser": { + "optional": true + } + } + }, "node_modules/@vitest/expect": { "version": "4.0.18", "dev": true, @@ -2902,6 +2943,25 @@ "node": ">=12" } }, + "node_modules/ast-v8-to-istanbul": { + "version": "0.3.12", + "resolved": "https://registry.npmjs.org/ast-v8-to-istanbul/-/ast-v8-to-istanbul-0.3.12.tgz", + "integrity": "sha512-BRRC8VRZY2R4Z4lFIL35MwNXmwVqBityvOIwETtsCSwvjl0IdgFsy9NhdaA6j74nUdtJJlIypeRhpDam19Wq3g==", + "dev": true, + "license": "MIT", + "dependencies": { + "@jridgewell/trace-mapping": "^0.3.31", + "estree-walker": "^3.0.3", + "js-tokens": "^10.0.0" + } + }, + "node_modules/ast-v8-to-istanbul/node_modules/js-tokens": { + "version": "10.0.0", + "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-10.0.0.tgz", + "integrity": "sha512-lM/UBzQmfJRo9ABXbPWemivdCW8V2G8FHaHdypQaIy523snUjog0W71ayWXTjiR+ixeMyVHN2XcpnTd/liPg/Q==", + "dev": true, + "license": "MIT" + }, "node_modules/astring": { "version": "1.9.0", "resolved": "https://registry.npmjs.org/astring/-/astring-1.9.0.tgz", @@ -6111,6 +6171,52 @@ "version": "2.0.0", "license": "ISC" }, + "node_modules/istanbul-lib-coverage": { + "version": "3.2.2", + "resolved": "https://registry.npmjs.org/istanbul-lib-coverage/-/istanbul-lib-coverage-3.2.2.tgz", + "integrity": "sha512-O8dpsF+r0WV/8MNRKfnmrtCWhuKjxrq2w+jpzBL5UZKTi2LeVWnWOmWRxFlesJONmc+wLAGvKQZEOanko0LFTg==", + "dev": true, + "license": "BSD-3-Clause", + "engines": { + "node": ">=8" + } + }, + "node_modules/istanbul-lib-report": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/istanbul-lib-report/-/istanbul-lib-report-3.0.1.tgz", + "integrity": "sha512-GCfE1mtsHGOELCU8e/Z7YWzpmybrx/+dSTfLrvY8qRmaY6zXTKWn6WQIjaAFw069icm6GVMNkgu0NzI4iPZUNw==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "istanbul-lib-coverage": "^3.0.0", + "make-dir": "^4.0.0", + "supports-color": "^7.1.0" + }, + "engines": { + "node": ">=10" + } + }, + "node_modules/istanbul-reports": { + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/istanbul-reports/-/istanbul-reports-3.2.0.tgz", + "integrity": "sha512-HGYWWS/ehqTV3xN10i23tkPkpH46MLCIMFNCaaKNavAXTF1RkqxawEPtnjnGZ6XKSInBKkiOA5BKS+aZiY3AvA==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "html-escaper": "^2.0.0", + "istanbul-lib-report": "^3.0.0" + }, + "engines": { + "node": ">=8" + } + }, + "node_modules/istanbul-reports/node_modules/html-escaper": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/html-escaper/-/html-escaper-2.0.2.tgz", + "integrity": "sha512-H2iMtd0I4Mt5eYiapRdIDjp+XzelXQ0tFE4JS7YFwFevXXMmOp9myNrUvCg0D6ws8iqkRPBfKHgbwig1SmlLfg==", + "dev": true, + "license": "MIT" + }, "node_modules/jackspeak": { "version": "3.4.3", "license": "BlueOak-1.0.0", @@ -6507,6 +6613,22 @@ "source-map-js": "^1.2.1" } }, + "node_modules/make-dir": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/make-dir/-/make-dir-4.0.0.tgz", + "integrity": "sha512-hXdUTZYIVOt1Ex//jAQi+wTZZpUpwBj/0QsOzqegb3rGMMeJiSEu5xLHnYfBrRV4RH2+OCSOO95Is/7x1WJ4bw==", + "dev": true, + "license": "MIT", + "dependencies": { + "semver": "^7.5.3" + }, + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/markdown-extensions": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/markdown-extensions/-/markdown-extensions-2.0.0.tgz", @@ -9528,6 +9650,19 @@ "inline-style-parser": "0.2.7" } }, + "node_modules/supports-color": { + "version": "7.2.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.2.0.tgz", + "integrity": "sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==", + "dev": true, + "license": "MIT", + "dependencies": { + "has-flag": "^4.0.0" + }, + "engines": { + "node": ">=8" + } + }, "node_modules/supports-hyperlinks": { "version": "4.4.0", "resolved": "https://registry.npmjs.org/supports-hyperlinks/-/supports-hyperlinks-4.4.0.tgz", @@ -11006,6 +11141,7 @@ "devDependencies": { "@erode-app/eslint-config": "*", "@types/node": "^24.12.0", + "@vitest/coverage-v8": "^4.0.18", "concurrently": "^9.2.1", "eslint": "^10.0.3", "prettier": "^3.7.3", @@ -11038,6 +11174,7 @@ "devDependencies": { "@erode-app/eslint-config": "*", "@types/node": "^24.12.0", + "@vitest/coverage-v8": "^4.0.18", "concurrently": "^9.2.1", "eslint": "^10.0.3", "prettier": "^3.7.3", diff --git a/package.json b/package.json index 5a3856c..7ef0e3b 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,7 @@ "build": "npm run build --workspace=packages/core && npm run build --workspace=packages/cli", "build:ci": "npm run build --workspace=packages/core", "test": "npm run test --workspaces --if-present", + "test:coverage": "npm run test:coverage --workspaces --if-present", "lint": "npm run lint --workspaces --if-present", "typecheck": "npm run typecheck --workspaces --if-present", "format": "prettier --write .", diff --git a/packages/cli/package.json b/packages/cli/package.json index 8256c94..5dec93d 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -21,6 +21,7 @@ "clean": "rimraf dist", "prebuild": "npm run clean", "test": "vitest run", + "test:coverage": "vitest run --coverage", "test:watch": "vitest", "check:ci": "concurrently --group \"npm:lint\" \"npm:typecheck\" \"npm:format:check\" \"npm:build\"" }, @@ -39,6 +40,7 @@ "devDependencies": { "@erode-app/eslint-config": "*", "@types/node": "^24.12.0", + "@vitest/coverage-v8": "^4.0.18", "concurrently": "^9.2.1", "eslint": "^10.0.3", "prettier": "^3.7.3", diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 43e1051..a9258c8 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -7,5 +7,12 @@ export default defineConfig({ }, exclude: ['dist/**', 'node_modules/**'], passWithNoTests: true, + coverage: { + provider: 'v8', + reporter: ['text', 'lcov', 'json-summary'], + reportsDirectory: 'coverage', + include: ['src/**/*.ts'], + exclude: ['src/**/__tests__/**', 'src/**/index.ts', 'src/**/*.d.ts'], + }, }, }); diff --git a/packages/core/package.json b/packages/core/package.json index b87a04d..cba3a14 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -31,6 +31,7 @@ "clean": "rimraf dist", "prebuild": "npm run clean", "test": "vitest run", + "test:coverage": "vitest run --coverage", "test:watch": "vitest", "typecheck": "tsc --noEmit", "generate:schema": "tsx scripts/generate-schema.ts" @@ -67,6 +68,7 @@ "devDependencies": { "@erode-app/eslint-config": "*", "@types/node": "^24.12.0", + "@vitest/coverage-v8": "^4.0.18", "concurrently": "^9.2.1", "eslint": "^10.0.3", "prettier": "^3.7.3", diff --git a/packages/core/vitest.config.ts b/packages/core/vitest.config.ts index 3bf66e4..70c484e 100644 --- a/packages/core/vitest.config.ts +++ b/packages/core/vitest.config.ts @@ -6,5 +6,12 @@ export default defineConfig({ ERODE_DEBUG_MODE: 'true', }, exclude: ['dist/**', 'node_modules/**'], + coverage: { + provider: 'v8', + reporter: ['text', 'lcov', 'json-summary'], + reportsDirectory: 'coverage', + include: ['src/**/*.ts'], + exclude: ['src/**/__tests__/**', 'src/**/index.ts', 'src/**/*.d.ts'], + }, }, }); From e61c49125b5e2618c33b3159a68af3af29ea4f74 Mon Sep 17 00:00:00 2001 From: Anders Hassis Date: Sun, 8 Mar 2026 11:10:36 +0100 Subject: [PATCH 2/4] test(core): extend test coverage across adapters, platforms, pipelines, providers, and utils Add tests for LikeC4 relationship queries and model-not-loaded guards, Bitbucket requestText/requestVoid methods, GitLab upsert/delete/close operations, config file error handling, pipeline component resolution and connection analysis, provider factory creation, and retry utility. --- package-lock.json | 29 ++- .../adapters/likec4/__tests__/adapter.test.ts | 60 ++++++ .../pipelines/__tests__/components.test.ts | 72 +++++++ .../pipelines/__tests__/connections.test.ts | 189 ++++++++++++++++++ .../bitbucket/__tests__/api-client.test.ts | 50 +++++ .../platforms/gitlab/__tests__/writer.test.ts | 86 ++++++++ .../__tests__/provider-factory.test.ts | 180 +++++++++++++++++ .../src/utils/__tests__/config-file.test.ts | 33 +++ .../core/src/utils/__tests__/retry.test.ts | 118 +++++++++++ 9 files changed, 814 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/pipelines/__tests__/components.test.ts create mode 100644 packages/core/src/pipelines/__tests__/connections.test.ts create mode 100644 packages/core/src/providers/__tests__/provider-factory.test.ts create mode 100644 packages/core/src/utils/__tests__/retry.test.ts diff --git a/package-lock.json b/package-lock.json index 2b1c1f0..2b32f94 100644 --- a/package-lock.json +++ b/package-lock.json @@ -85,7 +85,8 @@ }, "node_modules/@astrojs/compiler": { "version": "2.13.1", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/@astrojs/internal-helpers": { "version": "0.7.5", @@ -230,6 +231,7 @@ "resolved": "https://registry.npmjs.org/@astrojs/starlight/-/starlight-0.37.7.tgz", "integrity": "sha512-KyBnou8aKIlPJUSNx6a1SN7XyH22oj/VAvTGC+Edld4Bnei1A//pmCRTBvSrSeoGrdUjK0ErFUfaEhhO1bPfDg==", "license": "MIT", + "peer": true, "dependencies": { "@astrojs/markdown-remark": "^6.3.1", "@astrojs/mdx": "^4.2.3", @@ -310,6 +312,7 @@ "node_modules/@babel/core": { "version": "7.29.0", "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -1475,6 +1478,7 @@ "node_modules/@octokit/core": { "version": "7.0.6", "license": "MIT", + "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.3", @@ -2305,6 +2309,7 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-24.12.0.tgz", "integrity": "sha512-GYDxsZi3ChgmckRT9HPU0WEhKLP08ev/Yfcq2AstjrDASOYCSXeyjDsHg4v5t4jOj7cyDX3vmprafKlWIG9MXQ==", "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -2361,6 +2366,7 @@ "node_modules/@typescript-eslint/parser": { "version": "8.56.1", "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.56.1", "@typescript-eslint/types": "8.56.1", @@ -2767,6 +2773,7 @@ "node_modules/acorn": { "version": "8.16.0", "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2976,6 +2983,7 @@ "resolved": "https://registry.npmjs.org/astro/-/astro-5.18.0.tgz", "integrity": "sha512-CHiohwJIS4L0G6/IzE1Fx3dgWqXBCXus/od0eGUfxrZJD2um2pE7ehclMmgL/fXqbU7NfE1Ze2pq34h2QaA6iQ==", "license": "MIT", + "peer": true, "dependencies": { "@astrojs/compiler": "^2.13.0", "@astrojs/internal-helpers": "0.7.5", @@ -3300,6 +3308,7 @@ "node_modules/astro/node_modules/zod": { "version": "3.25.76", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } @@ -3481,6 +3490,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.9.0", "caniuse-lite": "^1.0.30001759", @@ -3923,6 +3933,7 @@ "integrity": "sha512-hr4ihw+DBqcvrsEDioRO31Z17x71pUYoNe/4h6Z0wB72p7MU7/9gH8Q3s12NFhHPfYBBOV3qyfUxmr/Yn3shnQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "env-paths": "^2.2.1", "import-fresh": "^3.3.0", @@ -4430,6 +4441,7 @@ "version": "0.27.3", "hasInstallScript": true, "license": "MIT", + "peer": true, "bin": { "esbuild": "bin/esbuild" }, @@ -4487,6 +4499,7 @@ "resolved": "https://registry.npmjs.org/eslint/-/eslint-10.0.3.tgz", "integrity": "sha512-COV33RzXZkqhG9P2rZCFl9ZmJ7WL+gQSCRzE7RhkbclbQPtLAWReL7ysA0Sh4c8Im2U9ynybdR56PV0XcKvqaQ==", "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.2", @@ -6234,6 +6247,7 @@ "version": "2.6.1", "devOptional": true, "license": "MIT", + "peer": true, "bin": { "jiti": "lib/jiti-cli.mjs" } @@ -6791,6 +6805,7 @@ "resolved": "https://registry.npmjs.org/marked/-/marked-15.0.12.tgz", "integrity": "sha512-8dD6FusOQSrpv9Z1rdNMdlSgQOIP880DHqnohobOmYLElGEqAL/JvxvuxZO16r4HtjTlfPRDC1hbvxC9dPN2nA==", "license": "MIT", + "peer": true, "bin": { "marked": "bin/marked.js" }, @@ -8381,6 +8396,7 @@ "node_modules/picomatch": { "version": "4.0.3", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -8452,6 +8468,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "nanoid": "^3.3.11", "picocolors": "^1.1.1", @@ -9090,6 +9107,7 @@ "node_modules/rollup": { "version": "4.59.0", "license": "MIT", + "peer": true, "dependencies": { "@types/estree": "1.0.8" }, @@ -9186,8 +9204,7 @@ }, "node_modules/scheduler": { "version": "0.27.0", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/semver": { "version": "7.7.4", @@ -9920,6 +9937,7 @@ "node_modules/typescript": { "version": "5.9.3", "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -10332,6 +10350,7 @@ "node_modules/vite": { "version": "7.3.1", "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.27.0", "fdir": "^6.5.0", @@ -10436,6 +10455,7 @@ "version": "4.0.18", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/expect": "4.0.18", "@vitest/mocker": "4.0.18", @@ -10921,6 +10941,7 @@ "version": "2.8.2", "devOptional": true, "license": "ISC", + "peer": true, "bin": { "yaml": "bin.mjs" }, @@ -10957,6 +10978,7 @@ "version": "8.18.0", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "fast-deep-equal": "^3.1.3", "fast-uri": "^3.0.1", @@ -11094,6 +11116,7 @@ "node_modules/zod": { "version": "4.3.6", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/packages/core/src/adapters/likec4/__tests__/adapter.test.ts b/packages/core/src/adapters/likec4/__tests__/adapter.test.ts index 801ef81..0db07f0 100644 --- a/packages/core/src/adapters/likec4/__tests__/adapter.test.ts +++ b/packages/core/src/adapters/likec4/__tests__/adapter.test.ts @@ -149,6 +149,24 @@ describe('LikeC4Adapter', () => { }); }); + describe('Relationship queries', () => { + it('should return all relationships via getAllRelationships', () => { + const relationships = adapter.getAllRelationships(); + expect(relationships).toHaveLength(7); + }); + + it('should return empty array for component with no outgoing relationships', () => { + const rels = adapter.getComponentRelationships('database'); + expect(rels).toEqual([]); + }); + + it('should return outgoing relationships with resolved targets', () => { + const rels = adapter.getComponentRelationships('frontend'); + expect(rels).toHaveLength(1); + expect(rels[0]?.target.id).toBe('api_gateway'); + }); + }); + describe('Repository-based lookup', () => { it('should find components by repository URL', () => { const component = adapter.findComponentByRepository( @@ -320,6 +338,48 @@ describe('LikeC4Adapter - Model not loaded guard', () => { expect(adapterError.adapterType).toBe('likec4'); } }); + + it('should throw AdapterError when getComponentRelationships is called before load', () => { + const adapter = new LikeC4Adapter(); + + expect(() => adapter.getComponentRelationships('any')).toThrow(AdapterError); + try { + adapter.getComponentRelationships('any'); + } catch (error) { + expect(error).toBeInstanceOf(AdapterError); + const adapterError = error as AdapterError; + expect(adapterError.code).toBe(ErrorCode.MODEL_NOT_INITIALIZED); + expect(adapterError.adapterType).toBe('likec4'); + } + }); + + it('should throw AdapterError when getComponentDependents is called before load', () => { + const adapter = new LikeC4Adapter(); + + expect(() => adapter.getComponentDependents('any')).toThrow(AdapterError); + try { + adapter.getComponentDependents('any'); + } catch (error) { + expect(error).toBeInstanceOf(AdapterError); + const adapterError = error as AdapterError; + expect(adapterError.code).toBe(ErrorCode.MODEL_NOT_INITIALIZED); + expect(adapterError.adapterType).toBe('likec4'); + } + }); + + it('should throw AdapterError when getAllRelationships is called before load', () => { + const adapter = new LikeC4Adapter(); + + expect(() => adapter.getAllRelationships()).toThrow(AdapterError); + try { + adapter.getAllRelationships(); + } catch (error) { + expect(error).toBeInstanceOf(AdapterError); + const adapterError = error as AdapterError; + expect(adapterError.code).toBe(ErrorCode.MODEL_NOT_INITIALIZED); + expect(adapterError.adapterType).toBe('likec4'); + } + }); }); describe('LikeC4Adapter - Component Exclusion', () => { diff --git a/packages/core/src/pipelines/__tests__/components.test.ts b/packages/core/src/pipelines/__tests__/components.test.ts new file mode 100644 index 0000000..efab92e --- /dev/null +++ b/packages/core/src/pipelines/__tests__/components.test.ts @@ -0,0 +1,72 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// ── Hoisted mock functions ──────────────────────────────────────────────── +const { mockLoadAndListComponents, mockValidatePath } = vi.hoisted(() => ({ + mockLoadAndListComponents: vi.fn(), + mockValidatePath: vi.fn(), +})); + +// ── Module mocks ────────────────────────────────────────────────────────── +vi.mock('../../adapters/adapter-factory.js', () => ({ + createAdapter: vi.fn(() => ({ + metadata: { displayName: 'LikeC4' }, + loadAndListComponents: mockLoadAndListComponents, + })), +})); + +vi.mock('../../utils/validation.js', () => ({ + validatePath: mockValidatePath, +})); + +vi.mock('../../utils/config.js', () => ({ + CONFIG: { + debug: { verbose: false }, + adapter: { likec4: { excludePaths: [], excludeTags: [] } }, + }, +})); + +// ── Imports (after mocks) ───────────────────────────────────────────────── +import { runComponents } from '../components.js'; +import { createAdapter } from '../../adapters/adapter-factory.js'; + +describe('runComponents', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns components from adapter.loadAndListComponents', async () => { + const fakeComponents = [ + { id: 'svc-a', title: 'Service A', kind: 'service', links: [], tags: [] }, + { id: 'svc-b', title: 'Service B', kind: 'database', links: [], tags: ['internal'] }, + ]; + mockLoadAndListComponents.mockResolvedValue(fakeComponents); + + const result = await runComponents({ modelPath: '/models/arch' }); + + expect(result).toEqual(fakeComponents); + }); + + it('calls validatePath with modelPath and "directory"', async () => { + mockLoadAndListComponents.mockResolvedValue([]); + + await runComponents({ modelPath: '/some/path' }); + + expect(mockValidatePath).toHaveBeenCalledWith('/some/path', 'directory'); + }); + + it('passes modelFormat to createAdapter', async () => { + mockLoadAndListComponents.mockResolvedValue([]); + + await runComponents({ modelPath: '/models/arch', modelFormat: 'structurizr' }); + + expect(createAdapter).toHaveBeenCalledWith('structurizr'); + }); + + it('passes modelPath to adapter.loadAndListComponents', async () => { + mockLoadAndListComponents.mockResolvedValue([]); + + await runComponents({ modelPath: '/models/arch' }); + + expect(mockLoadAndListComponents).toHaveBeenCalledWith('/models/arch'); + }); +}); diff --git a/packages/core/src/pipelines/__tests__/connections.test.ts b/packages/core/src/pipelines/__tests__/connections.test.ts new file mode 100644 index 0000000..a1c335c --- /dev/null +++ b/packages/core/src/pipelines/__tests__/connections.test.ts @@ -0,0 +1,189 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// ── Hoisted mock functions ──────────────────────────────────────────────── +const { + mockLoadFromPath, + mockFindAllComponentsByRepository, + mockGetComponentDependencies, + mockGetComponentDependents, + mockGetComponentRelationships, + mockValidatePath, + mockCreateAdapter, +} = vi.hoisted(() => ({ + mockLoadFromPath: vi.fn().mockResolvedValue({}), + mockFindAllComponentsByRepository: vi.fn(), + mockGetComponentDependencies: vi.fn().mockReturnValue([]), + mockGetComponentDependents: vi.fn().mockReturnValue([]), + mockGetComponentRelationships: vi.fn().mockReturnValue([]), + mockValidatePath: vi.fn(), + mockCreateAdapter: vi.fn(), +})); + +// ── Module mocks ────────────────────────────────────────────────────────── + +const mockAdapter = { + metadata: { displayName: 'LikeC4' }, + loadFromPath: mockLoadFromPath, + findAllComponentsByRepository: mockFindAllComponentsByRepository, + getComponentDependencies: mockGetComponentDependencies, + getComponentDependents: mockGetComponentDependents, + getComponentRelationships: mockGetComponentRelationships, +}; + +mockCreateAdapter.mockReturnValue(mockAdapter); + +vi.mock('../../adapters/adapter-factory.js', () => ({ + createAdapter: mockCreateAdapter, +})); + +vi.mock('../../utils/validation.js', () => ({ + validatePath: mockValidatePath, +})); + +vi.mock('../../utils/config.js', () => ({ + CONFIG: { + debug: { verbose: false }, + adapter: { likec4: { excludePaths: [], excludeTags: [] } }, + }, +})); + +import { runConnections } from '../connections.js'; +import type { ArchitecturalComponent } from '../../adapters/architecture-types.js'; + +// ── Test data builders ──────────────────────────────────────────────────── + +function makeComponent( + id = 'comp.api', + name = 'API Service' +): ArchitecturalComponent { + return { + id, + name, + type: 'service', + tags: [], + repository: 'https://github.com/org/repo', + }; +} + +function makeOptions(overrides: Partial<{ modelPath: string; modelFormat: string; repo: string }> = {}) { + return { + modelPath: '/path/to/model', + repo: 'org/repo', + ...overrides, + }; +} + +// ── Tests ───────────────────────────────────────────────────────────────── + +describe('runConnections', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockCreateAdapter.mockReturnValue(mockAdapter); + mockLoadFromPath.mockResolvedValue({}); + mockFindAllComponentsByRepository.mockReturnValue([]); + mockGetComponentDependencies.mockReturnValue([]); + mockGetComponentDependents.mockReturnValue([]); + mockGetComponentRelationships.mockReturnValue([]); + mockValidatePath.mockReturnValue(undefined); + }); + + it('returns connections for single component with deps, dependents, relationships', async () => { + const component = makeComponent('comp.api', 'API Service'); + const dep = makeComponent('comp.db', 'Database'); + const dependent = makeComponent('comp.web', 'Web App'); + + mockFindAllComponentsByRepository.mockReturnValue([component]); + mockGetComponentDependencies.mockReturnValue([dep]); + mockGetComponentDependents.mockReturnValue([dependent]); + mockGetComponentRelationships.mockReturnValue([ + { + target: { id: 'comp.db', name: 'Database', type: 'service', tags: [] }, + kind: 'uses', + title: 'reads data from', + }, + ]); + + const results = await runConnections(makeOptions()); + + expect(results).toHaveLength(1); + expect(results[0]?.component).toEqual({ + id: 'comp.api', + name: 'API Service', + type: 'service', + repository: 'https://github.com/org/repo', + }); + expect(results[0]?.dependencies).toEqual([ + { + id: 'comp.db', + name: 'Database', + type: 'service', + repository: 'https://github.com/org/repo', + }, + ]); + expect(results[0]?.dependents).toEqual([ + { + id: 'comp.web', + name: 'Web App', + type: 'service', + repository: 'https://github.com/org/repo', + }, + ]); + expect(results[0]?.relationships).toEqual([ + { + targetId: 'comp.db', + targetName: 'Database', + kind: 'uses', + title: 'reads data from', + }, + ]); + }); + + it('returns connections for multiple components', async () => { + const comp1 = makeComponent('comp.api', 'API Service'); + const comp2 = makeComponent('comp.worker', 'Worker Service'); + + mockFindAllComponentsByRepository.mockReturnValue([comp1, comp2]); + + const results = await runConnections(makeOptions()); + + expect(results).toHaveLength(2); + expect(results[0]?.component.id).toBe('comp.api'); + expect(results[1]?.component.id).toBe('comp.worker'); + }); + + it('returns [] and calls progress.warn when no components found', async () => { + mockFindAllComponentsByRepository.mockReturnValue([]); + + const mockProgress = { + section: vi.fn(), + start: vi.fn(), + succeed: vi.fn(), + fail: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + }; + + const results = await runConnections(makeOptions(), mockProgress); + + expect(results).toEqual([]); + expect(mockProgress.warn).toHaveBeenCalledWith( + 'No components found for repository: org/repo' + ); + }); + + it('passes modelFormat to createAdapter', async () => { + mockFindAllComponentsByRepository.mockReturnValue([]); + + await runConnections(makeOptions({ modelFormat: 'structurizr' })); + + expect(mockCreateAdapter).toHaveBeenCalledWith('structurizr'); + }); + + it('calls validatePath with modelPath and "directory"', async () => { + mockFindAllComponentsByRepository.mockReturnValue([]); + + await runConnections(makeOptions({ modelPath: '/custom/model/path' })); + + expect(mockValidatePath).toHaveBeenCalledWith('/custom/model/path', 'directory'); + }); +}); diff --git a/packages/core/src/platforms/bitbucket/__tests__/api-client.test.ts b/packages/core/src/platforms/bitbucket/__tests__/api-client.test.ts index 454cff3..8040668 100644 --- a/packages/core/src/platforms/bitbucket/__tests__/api-client.test.ts +++ b/packages/core/src/platforms/bitbucket/__tests__/api-client.test.ts @@ -170,4 +170,54 @@ describe('BitbucketApiClient', () => { } }); }); + + describe('requestText', () => { + it('should return text on success', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce( + mockFetchResponse('diff content here') + ); + + const result = await client.requestText('/repos/org/repo/diff'); + + expect(result).toBe('diff content here'); + }); + + it('should throw ApiError on non-ok response', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce( + mockFetchResponse('Not found', false, 404) + ); + + try { + await client.requestText('/repos/org/repo/diff'); + expect.fail('Expected ApiError'); + } catch (error) { + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).statusCode).toBe(404); + } + }); + }); + + describe('requestVoid', () => { + it('should resolve void on success', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce( + mockFetchResponse('', true, 204) + ); + + const result = await client.requestVoid('/repos/org/repo/approve', { + method: 'POST', + }); + + expect(result).toBeUndefined(); + }); + + it('should throw ApiError on non-ok response', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce( + mockFetchResponse('Server Error', false, 500) + ); + + await expect( + client.requestVoid('/repos/org/repo/approve', { method: 'POST' }) + ).rejects.toThrow(ApiError); + }); + }); }); diff --git a/packages/core/src/platforms/gitlab/__tests__/writer.test.ts b/packages/core/src/platforms/gitlab/__tests__/writer.test.ts index 6f29d12..f1c4888 100644 --- a/packages/core/src/platforms/gitlab/__tests__/writer.test.ts +++ b/packages/core/src/platforms/gitlab/__tests__/writer.test.ts @@ -10,6 +10,9 @@ const mockMrAll = vi.fn(); const mockMrEdit = vi.fn(); const mockMrCreate = vi.fn(); const mockMrNotesCreate = vi.fn(); +const mockMrNotesAll = vi.fn(); +const mockMrNotesEdit = vi.fn(); +const mockMrNotesRemove = vi.fn(); vi.mock('@gitbeaker/rest', () => ({ Gitlab: class MockGitlab { @@ -27,6 +30,9 @@ vi.mock('@gitbeaker/rest', () => ({ }; MergeRequestNotes = { create: mockMrNotesCreate, + all: mockMrNotesAll, + edit: mockMrNotesEdit, + remove: mockMrNotesRemove, }; }, })); @@ -279,5 +285,85 @@ describe('GitLabWriter', () => { await expect(writer.commentOnChangeRequest(ref, 'test')).rejects.toBe(erodeError); }); + + it('should update existing note via .edit when upsertMarker matches', async () => { + mockMrNotesAll.mockResolvedValue([{ id: 10, body: 'old content ' }]); + mockMrNotesEdit.mockResolvedValue({}); + const ref = makeRef(); + + await writer.commentOnChangeRequest(ref, 'new content', { + upsertMarker: '', + }); + + expect(mockMrNotesEdit).toHaveBeenCalledWith('org/repo', 42, 10, { body: 'new content' }); + expect(mockMrNotesCreate).not.toHaveBeenCalled(); + }); + + it('should create new note when upsertMarker not found', async () => { + mockMrNotesAll.mockResolvedValue([{ id: 10, body: 'unrelated note' }]); + mockMrNotesCreate.mockResolvedValue({}); + const ref = makeRef(); + + await writer.commentOnChangeRequest(ref, 'new content', { + upsertMarker: '', + }); + + expect(mockMrNotesCreate).toHaveBeenCalledWith('org/repo', 42, 'new content'); + expect(mockMrNotesEdit).not.toHaveBeenCalled(); + }); + + it('should create new note when no notes exist', async () => { + mockMrNotesAll.mockResolvedValue([]); + mockMrNotesCreate.mockResolvedValue({}); + const ref = makeRef(); + + await writer.commentOnChangeRequest(ref, 'new content', { + upsertMarker: '', + }); + + expect(mockMrNotesCreate).toHaveBeenCalledWith('org/repo', 42, 'new content'); + }); + }); + + describe('deleteComment', () => { + it('should remove note matching marker', async () => { + mockMrNotesAll.mockResolvedValue([{ id: 20, body: ' content' }]); + mockMrNotesRemove.mockResolvedValue({}); + const ref = makeRef(); + + await writer.deleteComment(ref, ''); + + expect(mockMrNotesRemove).toHaveBeenCalledWith('org/repo', 42, 20); + }); + + it('should no-op when no note matches marker', async () => { + mockMrNotesAll.mockResolvedValue([{ id: 20, body: 'unrelated' }]); + const ref = makeRef(); + + await writer.deleteComment(ref, ''); + + expect(mockMrNotesRemove).not.toHaveBeenCalled(); + }); + }); + + describe('closeChangeRequest', () => { + it('should close open MR via .edit with stateEvent close', async () => { + mockMrAll.mockResolvedValue([ + { iid: 42, web_url: 'https://gitlab.com/org/repo/-/merge_requests/42' }, + ]); + mockMrEdit.mockResolvedValue({}); + + await writer.closeChangeRequest('feature-branch'); + + expect(mockMrEdit).toHaveBeenCalledWith('org/repo', 42, { stateEvent: 'close' }); + }); + + it('should no-op when no open MR exists', async () => { + mockMrAll.mockResolvedValue([]); + + await writer.closeChangeRequest('feature-branch'); + + expect(mockMrEdit).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/core/src/providers/__tests__/provider-factory.test.ts b/packages/core/src/providers/__tests__/provider-factory.test.ts new file mode 100644 index 0000000..8b1dbce --- /dev/null +++ b/packages/core/src/providers/__tests__/provider-factory.test.ts @@ -0,0 +1,180 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { ErodeError, ErrorCode } from '../../errors.js'; + +const { + mockGeminiInstance, + mockOpenAIInstance, + mockAnthropicInstance, + MockGeminiProvider, + MockOpenAIProvider, + MockAnthropicProvider, + mockConfig, +} = vi.hoisted(() => { + const geminiInstance = { type: 'gemini' }; + const openaiInstance = { type: 'openai' }; + const anthropicInstance = { type: 'anthropic' }; + return { + mockGeminiInstance: geminiInstance, + mockOpenAIInstance: openaiInstance, + mockAnthropicInstance: anthropicInstance, + MockGeminiProvider: vi.fn(function () { + return geminiInstance; + }), + MockOpenAIProvider: vi.fn(function () { + return openaiInstance; + }), + MockAnthropicProvider: vi.fn(function () { + return anthropicInstance; + }), + mockConfig: { + ai: { provider: 'gemini' as 'gemini' | 'openai' | 'anthropic' }, + gemini: { + apiKey: 'test-gemini-key' as string | undefined, + fastModel: 'gemini-flash', + advancedModel: 'gemini-pro', + }, + openai: { + apiKey: 'test-openai-key' as string | undefined, + fastModel: 'gpt-4.1-mini', + advancedModel: 'gpt-4.1', + }, + anthropic: { + apiKey: 'test-anthropic-key' as string | undefined, + fastModel: 'claude-haiku', + advancedModel: 'claude-sonnet', + }, + }, + }; +}); + +vi.mock('../gemini/provider.js', () => ({ + GeminiProvider: MockGeminiProvider, +})); +vi.mock('../openai/provider.js', () => ({ + OpenAIProvider: MockOpenAIProvider, +})); +vi.mock('../anthropic/provider.js', () => ({ + AnthropicProvider: MockAnthropicProvider, +})); +vi.mock('../../utils/config.js', () => ({ + CONFIG: mockConfig, + ENV_VAR_NAMES: { + geminiApiKey: 'ERODE_GEMINI_API_KEY', + openaiApiKey: 'ERODE_OPENAI_API_KEY', + anthropicApiKey: 'ERODE_ANTHROPIC_API_KEY', + }, + RC_FILENAME: '.eroderc.json', +})); + +import { createAIProvider } from '../provider-factory.js'; +import { GeminiProvider } from '../gemini/provider.js'; +import { OpenAIProvider } from '../openai/provider.js'; +import { AnthropicProvider } from '../anthropic/provider.js'; + +describe('createAIProvider', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockConfig.ai.provider = 'gemini'; + mockConfig.gemini.apiKey = 'test-gemini-key'; + mockConfig.gemini.fastModel = 'gemini-flash'; + mockConfig.gemini.advancedModel = 'gemini-pro'; + mockConfig.openai.apiKey = 'test-openai-key'; + mockConfig.openai.fastModel = 'gpt-4.1-mini'; + mockConfig.openai.advancedModel = 'gpt-4.1'; + mockConfig.anthropic.apiKey = 'test-anthropic-key'; + mockConfig.anthropic.fastModel = 'claude-haiku'; + mockConfig.anthropic.advancedModel = 'claude-sonnet'; + }); + + it('should return GeminiProvider when provider is gemini and key is set', () => { + mockConfig.ai.provider = 'gemini'; + + const result = createAIProvider(); + + expect(result).toBe(mockGeminiInstance); + expect(GeminiProvider).toHaveBeenCalledWith({ + apiKey: 'test-gemini-key', + fastModel: 'gemini-flash', + advancedModel: 'gemini-pro', + }); + }); + + it('should return OpenAIProvider when provider is openai and key is set', () => { + mockConfig.ai.provider = 'openai'; + + const result = createAIProvider(); + + expect(result).toBe(mockOpenAIInstance); + expect(OpenAIProvider).toHaveBeenCalledWith({ + apiKey: 'test-openai-key', + fastModel: 'gpt-4.1-mini', + advancedModel: 'gpt-4.1', + }); + }); + + it('should return AnthropicProvider when provider is anthropic and key is set', () => { + mockConfig.ai.provider = 'anthropic'; + + const result = createAIProvider(); + + expect(result).toBe(mockAnthropicInstance); + expect(AnthropicProvider).toHaveBeenCalledWith({ + apiKey: 'test-anthropic-key', + fastModel: 'claude-haiku', + advancedModel: 'claude-sonnet', + }); + }); + + it('should throw AUTH_KEY_MISSING when gemini API key is missing', () => { + mockConfig.ai.provider = 'gemini'; + mockConfig.gemini.apiKey = undefined; + + expect(() => createAIProvider()).toThrow(ErodeError); + try { + createAIProvider(); + } catch (error) { + expect(error).toBeInstanceOf(ErodeError); + expect((error as ErodeError).code).toBe(ErrorCode.AUTH_KEY_MISSING); + } + }); + + it('should throw AUTH_KEY_MISSING when openai API key is missing', () => { + mockConfig.ai.provider = 'openai'; + mockConfig.openai.apiKey = undefined; + + expect(() => createAIProvider()).toThrow(ErodeError); + try { + createAIProvider(); + } catch (error) { + expect(error).toBeInstanceOf(ErodeError); + expect((error as ErodeError).code).toBe(ErrorCode.AUTH_KEY_MISSING); + } + }); + + it('should throw AUTH_KEY_MISSING when anthropic API key is missing', () => { + mockConfig.ai.provider = 'anthropic'; + mockConfig.anthropic.apiKey = undefined; + + expect(() => createAIProvider()).toThrow(ErodeError); + try { + createAIProvider(); + } catch (error) { + expect(error).toBeInstanceOf(ErodeError); + expect((error as ErodeError).code).toBe(ErrorCode.AUTH_KEY_MISSING); + } + }); + + it('should pass fastModel and advancedModel to provider constructors', () => { + mockConfig.ai.provider = 'gemini'; + mockConfig.gemini.fastModel = 'custom-fast'; + mockConfig.gemini.advancedModel = 'custom-advanced'; + + createAIProvider(); + + expect(GeminiProvider).toHaveBeenCalledWith({ + apiKey: 'test-gemini-key', + fastModel: 'custom-fast', + advancedModel: 'custom-advanced', + }); + }); +}); diff --git a/packages/core/src/utils/__tests__/config-file.test.ts b/packages/core/src/utils/__tests__/config-file.test.ts index 241190e..31b59d7 100644 --- a/packages/core/src/utils/__tests__/config-file.test.ts +++ b/packages/core/src/utils/__tests__/config-file.test.ts @@ -18,6 +18,7 @@ import { loadConfigFromFile, loadConfigFromEnv, } from '../config.js'; +import { ConfigurationError } from '../../errors.js'; const mockExistsSync = vi.mocked(fs.existsSync); const mockReadFileSync = vi.mocked(fs.readFileSync); @@ -214,4 +215,36 @@ describe('config file support', () => { } }); }); + + describe('loadConfigFromFile error handling', () => { + it('throws ConfigurationError when file contains invalid JSON', () => { + mockReadFileSync.mockReturnValue('not valid json {'); + + expect(() => loadConfigFromFile('/fake/path')).toThrow(ConfigurationError); + expect(() => { + mockReadFileSync.mockReturnValue('not valid json {'); + return loadConfigFromFile('/fake/path'); + }).toThrow(/Failed to load config file/); + }); + + it('throws ConfigurationError when file contains an array', () => { + mockReadFileSync.mockReturnValue('[1, 2, 3]'); + + expect(() => loadConfigFromFile('/fake/path')).toThrow(ConfigurationError); + expect(() => { + mockReadFileSync.mockReturnValue('[1, 2, 3]'); + return loadConfigFromFile('/fake/path'); + }).toThrow(/must contain a JSON object/); + }); + + it('throws ConfigurationError when file contains null', () => { + mockReadFileSync.mockReturnValue('null'); + + expect(() => loadConfigFromFile('/fake/path')).toThrow(ConfigurationError); + expect(() => { + mockReadFileSync.mockReturnValue('null'); + return loadConfigFromFile('/fake/path'); + }).toThrow(/must contain a JSON object/); + }); + }); }); diff --git a/packages/core/src/utils/__tests__/retry.test.ts b/packages/core/src/utils/__tests__/retry.test.ts new file mode 100644 index 0000000..19f56d6 --- /dev/null +++ b/packages/core/src/utils/__tests__/retry.test.ts @@ -0,0 +1,118 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +const mockConfig = vi.hoisted(() => ({ + debug: { verbose: false }, +})); + +vi.mock('../config.js', () => ({ + CONFIG: mockConfig, +})); + +import { withRetry } from '../retry.js'; +import { ErodeError, ErrorCode } from '../../errors.js'; + +const FAST_POLICY = { + retries: 2, + initialDelay: 1, + delayCap: 10, + useExponentialBackoff: false, +}; + +describe('withRetry', () => { + beforeEach(() => { + vi.restoreAllMocks(); + mockConfig.debug.verbose = false; + }); + + afterEach(() => { + mockConfig.debug.verbose = false; + }); + + it('returns result on first success without retrying', async () => { + const operation = vi.fn().mockResolvedValue('ok'); + + const result = await withRetry(operation, FAST_POLICY); + + expect(result).toBe('ok'); + expect(operation).toHaveBeenCalledTimes(1); + }); + + it('retries on recoverable error, then succeeds', async () => { + const operation = vi + .fn() + .mockRejectedValueOnce(new Error('transient failure')) + .mockResolvedValue('recovered'); + + const result = await withRetry(operation, FAST_POLICY); + + expect(result).toBe('recovered'); + expect(operation).toHaveBeenCalledTimes(2); + }); + + it('throws immediately when shouldRetry returns false', async () => { + const error = new Error('not retryable'); + const operation = vi.fn().mockRejectedValue(error); + + await expect( + withRetry(operation, { + ...FAST_POLICY, + shouldRetry: () => false, + }) + ).rejects.toThrow('not retryable'); + + expect(operation).toHaveBeenCalledTimes(1); + }); + + it('throws immediately for non-recoverable ErodeError without custom shouldRetry', async () => { + const error = new ErodeError( + 'fail', + ErrorCode.CONFIG_INVALID, + undefined, + {}, + false + ); + const operation = vi.fn().mockRejectedValue(error); + + await expect(withRetry(operation, FAST_POLICY)).rejects.toThrow(error); + expect(operation).toHaveBeenCalledTimes(1); + }); + + it('throws last error after exhausting all retries', async () => { + const operation = vi.fn().mockRejectedValue(new Error('always fails')); + + await expect( + withRetry(operation, { ...FAST_POLICY, retries: 1 }) + ).rejects.toThrow('always fails'); + + // 1 initial attempt + 1 retry = 2 calls + expect(operation).toHaveBeenCalledTimes(2); + }); + + it('logs retry message when CONFIG.debug.verbose is true', async () => { + mockConfig.debug.verbose = true; + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const operation = vi + .fn() + .mockRejectedValueOnce(new Error('transient')) + .mockResolvedValue('ok'); + + await withRetry(operation, FAST_POLICY); + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('[Retry]') + ); + }); + + it('does not log when verbose is false', async () => { + mockConfig.debug.verbose = false; + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const operation = vi + .fn() + .mockRejectedValueOnce(new Error('transient')) + .mockResolvedValue('ok'); + + await withRetry(operation, FAST_POLICY); + + expect(consoleSpy).not.toHaveBeenCalled(); + }); +}); From d16253d8324d7233b2ebc142eed1dbc3e33253ed Mon Sep 17 00:00:00 2001 From: Anders Hassis Date: Sun, 8 Mar 2026 11:30:49 +0100 Subject: [PATCH 3/4] refactor(core): remove low-value tests and redundant comments Remove 25 tests that provide no meaningful safety: tautological assertions, TypeScript type-system duplicates, pure mock-call verification, and exact duplicates. Remove ~46 comments that restate what the next line of code does and redundant JSDoc that merely echoes property names. Also fix pre-existing lint errors in api-client.test.ts and retry.test.ts. --- packages/core/src/adapters/base-patcher.ts | 7 -- .../__tests__/adapter-url-validation.test.ts | 89 ------------------- .../likec4/__tests__/integration.test.ts | 12 +-- packages/core/src/adapters/model-patcher.ts | 2 - .../structurizr/__tests__/integration.test.ts | 8 -- .../core/src/adapters/structurizr/adapter.ts | 11 --- .../core/src/adapters/structurizr/patcher.ts | 3 - packages/core/src/analysis/analysis-types.ts | 21 ++--- .../src/pipelines/__tests__/analyze.test.ts | 12 --- .../pipelines/__tests__/components.test.ts | 25 ------ .../pipelines/__tests__/connections.test.ts | 29 ++---- .../pipelines/__tests__/pr-creation.test.ts | 12 --- .../__tests__/platform-factory.test.ts | 5 -- .../platforms/__tests__/reader-contract.ts | 42 --------- .../bitbucket/__tests__/api-client.test.ts | 12 +-- .../bitbucket/__tests__/reader.test.ts | 7 -- .../core/src/platforms/bitbucket/writer.ts | 5 -- .../platforms/gitlab/__tests__/reader.test.ts | 7 -- .../anthropic/__tests__/provider.test.ts | 5 -- .../core/src/providers/anthropic/provider.ts | 1 - .../gemini/__tests__/provider.test.ts | 5 -- .../openai/__tests__/provider.test.ts | 5 -- .../src/utils/__tests__/config-file.test.ts | 42 +-------- .../core/src/utils/__tests__/retry.test.ts | 38 ++++---- packages/core/src/utils/validation.ts | 1 - 25 files changed, 31 insertions(+), 375 deletions(-) delete mode 100644 packages/core/src/adapters/likec4/__tests__/adapter-url-validation.test.ts diff --git a/packages/core/src/adapters/base-patcher.ts b/packages/core/src/adapters/base-patcher.ts index d9594aa..c9bf462 100644 --- a/packages/core/src/adapters/base-patcher.ts +++ b/packages/core/src/adapters/base-patcher.ts @@ -266,7 +266,6 @@ export abstract class BasePatcher implements ModelPatcher { const contentLines = content.split('\n'); let insertIndex = -1; - // Find the model block and its closing brace let inModel = false; let braceDepth = 0; for (let i = 0; i < contentLines.length; i++) { @@ -306,18 +305,15 @@ export abstract class BasePatcher implements ModelPatcher { return content + '\n' + lines.join('\n') + '\n'; } - // Detect indentation from the line above the closing brace const lineAbove = contentLines[insertIndex - 1] ?? ''; const match = /^(\s*)/.exec(lineAbove); const indent = match?.[1] ?? this.defaultIndent; - // Re-indent lines to match const indentedLines = lines.map((line) => { const trimmed = line.trimStart(); return indent + trimmed; }); - // Insert lines before the closing brace contentLines.splice(insertIndex, 0, '', ...indentedLines); return contentLines.join('\n'); @@ -342,7 +338,6 @@ export function quickValidatePatch( patched: string, insertedLines: string[] ): boolean { - // Check all original non-empty lines are preserved const originalLines = original.split('\n').filter((l) => l.trim().length > 0); for (const line of originalLines) { if (!patched.includes(line)) { @@ -353,7 +348,6 @@ export function quickValidatePatch( } } - // Check inserted lines are present for (const line of insertedLines) { if (!patched.includes(line.trim())) { if (CONFIG.debug.verbose) { @@ -363,7 +357,6 @@ export function quickValidatePatch( } } - // Check brace balance const openBraces = (patched.match(/\{/g) ?? []).length; const closeBraces = (patched.match(/\}/g) ?? []).length; if (openBraces !== closeBraces) { diff --git a/packages/core/src/adapters/likec4/__tests__/adapter-url-validation.test.ts b/packages/core/src/adapters/likec4/__tests__/adapter-url-validation.test.ts deleted file mode 100644 index e6c144d..0000000 --- a/packages/core/src/adapters/likec4/__tests__/adapter-url-validation.test.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { describe, it, expect, beforeEach } from 'vitest'; -import { LikeC4Adapter } from '../adapter.js'; -import type { ArchitecturalComponent } from '../../architecture-types.js'; -import { createMockModel, type MockLikeC4Model } from './fixtures/mock-model.js'; - -class TestLikeC4Adapter extends LikeC4Adapter { - public setMockModel(model: MockLikeC4Model): void { - const adapter = this as unknown as { - model: MockLikeC4Model; - componentIndex: unknown; - relationships: unknown; - }; - adapter.model = model; - - const components = this.extractComponents(); - const relationships = this.extractRelationships(); - adapter.relationships = relationships; - adapter.componentIndex = this.buildComponentIndex(components); - } - - public override extractComponents() { - return super.extractComponents(); - } - - public override extractRelationships() { - return super.extractRelationships(); - } - - public override buildComponentIndex(components: ArchitecturalComponent[]) { - return super.buildComponentIndex(components); - } -} - -describe('LikeC4Adapter - URL spoofing protection', () => { - let adapter: TestLikeC4Adapter; - - beforeEach(async () => { - const configModule = await import('../../../utils/config.js'); - configModule.CONFIG.adapter.likec4.excludePaths = []; - configModule.CONFIG.adapter.likec4.excludeTags = []; - - adapter = new TestLikeC4Adapter(); - adapter.setMockModel(createMockModel()); - }); - - it('should reject URLs from lookalike domains', () => { - const spoofedModel: MockLikeC4Model = { - elements: () => [ - { - id: 'spoofed_service', - title: 'Spoofed Service', - kind: 'service', - tags: [], - links: ['https://evil-github.com/owner/repo'], - }, - ], - relationships: () => [], - }; - adapter.setMockModel(spoofedModel); - const components = adapter.extractComponents(); - const spoofed = components.find((c) => c.id === 'spoofed_service'); - expect(spoofed?.repository).toBeUndefined(); - }); - - it('should reject URLs with github.com as query parameter', () => { - const spoofedModel: MockLikeC4Model = { - elements: () => [ - { - id: 'query_spoofed', - title: 'Query Spoofed', - kind: 'service', - tags: [], - links: ['https://evil.com?redirect=github.com/owner/repo'], - }, - ], - relationships: () => [], - }; - adapter.setMockModel(spoofedModel); - const components = adapter.extractComponents(); - const spoofed = components.find((c) => c.id === 'query_spoofed'); - expect(spoofed?.repository).toBeUndefined(); - }); - - it('should accept legitimate github.com URLs', () => { - const components = adapter.extractComponents(); - const frontend = components.find((c) => c.id === 'frontend'); - expect(frontend?.repository).toBe('https://github.com/example/frontend'); - }); -}); diff --git a/packages/core/src/adapters/likec4/__tests__/integration.test.ts b/packages/core/src/adapters/likec4/__tests__/integration.test.ts index 48ee8df..a9b9872 100644 --- a/packages/core/src/adapters/likec4/__tests__/integration.test.ts +++ b/packages/core/src/adapters/likec4/__tests__/integration.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, beforeEach } from 'vitest'; import { LikeC4Adapter } from '../adapter.js'; import path from 'path'; import { fileURLToPath } from 'url'; @@ -21,14 +21,4 @@ describe('LikeC4Adapter Integration', () => { }); runAdapterContractTests(() => adapter); - - describe('LikeC4-specific', () => { - it('should handle file operations without errors', async () => { - const freshAdapter = new LikeC4Adapter(); - const startTime = performance.now(); - await freshAdapter.loadFromPath(fixtureWorkspace); - const loadTime = performance.now() - startTime; - expect(loadTime).toBeGreaterThan(0); - }); - }); }); diff --git a/packages/core/src/adapters/model-patcher.ts b/packages/core/src/adapters/model-patcher.ts index 7035ea3..55584c1 100644 --- a/packages/core/src/adapters/model-patcher.ts +++ b/packages/core/src/adapters/model-patcher.ts @@ -24,9 +24,7 @@ export interface PatchResult { filePath: string; /** Absolute path to the patched file, for local writes */ absolutePath: string; - /** Complete patched file content */ content: string; - /** Lines that were inserted */ insertedLines: string[]; /** Only the relationship DSL lines (subset of insertedLines, excludes component DSL) */ relationshipLines?: string[]; diff --git a/packages/core/src/adapters/structurizr/__tests__/integration.test.ts b/packages/core/src/adapters/structurizr/__tests__/integration.test.ts index 1276072..a525fc1 100644 --- a/packages/core/src/adapters/structurizr/__tests__/integration.test.ts +++ b/packages/core/src/adapters/structurizr/__tests__/integration.test.ts @@ -23,14 +23,6 @@ describe('StructurizrAdapter Integration', () => { runAdapterContractTests(() => adapter); describe('Structurizr-specific', () => { - it('should handle file operations without errors', async () => { - const freshAdapter = new StructurizrAdapter(); - const startTime = performance.now(); - await freshAdapter.loadFromPath(fixtureWorkspace); - const loadTime = performance.now() - startTime; - expect(loadTime).toBeGreaterThan(0); - }); - it('should filter built-in Structurizr tags', () => { const components = adapter.getAllComponents(); for (const component of components) { diff --git a/packages/core/src/adapters/structurizr/adapter.ts b/packages/core/src/adapters/structurizr/adapter.ts index e9f4ce4..8c58e71 100644 --- a/packages/core/src/adapters/structurizr/adapter.ts +++ b/packages/core/src/adapters/structurizr/adapter.ts @@ -164,7 +164,6 @@ export class StructurizrAdapter implements ArchitectureModelAdapter { }); }; - // Collect from all elements this.walkElements((element) => { if (element.relationships) { for (const rel of element.relationships) { @@ -173,7 +172,6 @@ export class StructurizrAdapter implements ArchitectureModelAdapter { } }); - // Collect model-level relationships const modelRels = this.workspace.model?.relationships; if (modelRels) { for (const rel of modelRels) { @@ -307,8 +305,6 @@ export class StructurizrAdapter implements ArchitectureModelAdapter { return false; } - // --- Private helpers --- - private resolveWorkspacePath(inputPath: string): string { try { if (!statSync(inputPath).isDirectory()) { @@ -341,10 +337,8 @@ export class StructurizrAdapter implements ArchitectureModelAdapter { const localId = element.id ?? this.toSnakeCase(element.name ?? ''); const resolvedId = this.resolveElementId(element, localId, parentPath); - // Map local identifier to resolved ID map.set(localId, resolvedId); - // Map full dotted path to resolved ID const dottedPath = parentPath ? `${parentPath}.${localId}` : localId; if (dottedPath !== localId) { map.set(dottedPath, resolvedId); @@ -364,7 +358,6 @@ export class StructurizrAdapter implements ArchitectureModelAdapter { const resolvedId = this.resolveElementId(element, localId, parentPath); callback(element, parentPath); - // Walk children const sys = element as StructurizrSoftwareSystem; if (sys.containers) walk(sys.containers, resolvedId); const container = element as StructurizrContainer; @@ -386,13 +379,10 @@ export class StructurizrAdapter implements ArchitectureModelAdapter { return element.properties['erode.id']; } - // Priority 2: DSL identifier (already in localId) - // Priority 3: snake_case fallback (already in localId from toSnakeCase) return parentPath ? `${parentPath}.${localId}` : localId; } private resolveIdentifier(ref: string): string | undefined { - // Try direct lookup const resolved = this.identifierMap.get(ref); if (resolved) return resolved; @@ -426,7 +416,6 @@ export class StructurizrAdapter implements ArchitectureModelAdapter { const container = element as StructurizrContainer; if (container.components) return 'container'; - // Check if this is a person if ( this.workspace?.model?.people?.some((p) => p.id === element.id && p.name === element.name) ) { diff --git a/packages/core/src/adapters/structurizr/patcher.ts b/packages/core/src/adapters/structurizr/patcher.ts index d1d62fa..083ec71 100644 --- a/packages/core/src/adapters/structurizr/patcher.ts +++ b/packages/core/src/adapters/structurizr/patcher.ts @@ -38,7 +38,6 @@ export class StructurizrPatcher extends BasePatcher { protected findTargetFile(modelPath: string): string | null { const resolvedPath = resolve(modelPath); - // If it's a file, use it directly try { if (!statSync(resolvedPath).isDirectory()) { return resolvedPath; @@ -48,12 +47,10 @@ export class StructurizrPatcher extends BasePatcher { return null; } - // Look for .dsl files in the directory const dslFiles = readdirSync(resolvedPath) .filter((f) => f.endsWith('.dsl')) .map((f) => join(resolvedPath, f)); - // Prefer workspace.dsl const workspaceDsl = dslFiles.find((f) => f.endsWith('workspace.dsl')); if (workspaceDsl) return workspaceDsl; diff --git a/packages/core/src/analysis/analysis-types.ts b/packages/core/src/analysis/analysis-types.ts index bc4251e..a7f8f2e 100644 --- a/packages/core/src/analysis/analysis-types.ts +++ b/packages/core/src/analysis/analysis-types.ts @@ -48,17 +48,12 @@ export interface DependencyExtractionPromptData { * Violation found in a PR with commit context */ export interface DriftViolation { - /** Severity of the violation */ severity: 'high' | 'medium' | 'low'; - /** Description of the violation */ description: string; - /** File where the violation occurred */ file?: string | null; - /** Line number if applicable */ line?: number | null; /** Commit SHA where this violation was introduced */ commit?: string | null; - /** Suggested fix */ suggestion?: string; } /** @@ -86,15 +81,13 @@ export interface ChangeRequestMetadata { * Result of analyzing a change request */ export interface DriftAnalysisResult { - /** Whether any violations were found */ hasViolations: boolean; - /** List of violations across all commits */ + /** Violations across all commits */ violations: DriftViolation[]; - /** Architectural improvements or positive findings */ + /** Positive architectural findings */ improvements?: string[]; - /** Warnings that are not critical violations */ + /** Not critical violations */ warnings?: string[]; - /** Summary of the analysis */ summary: string; /** Recommended model updates */ modelUpdates?: { @@ -104,9 +97,7 @@ export interface DriftAnalysisResult { relationships?: StructuredRelationship[]; newComponents?: NewComponent[]; }; - /** Change request metadata */ metadata: ChangeRequestMetadata; - /** Component that was analyzed */ component: ArchitecturalComponent; /** Aggregated dependency changes across all commits */ dependencyChanges: DependencyExtractionResult; @@ -115,11 +106,9 @@ export interface DriftAnalysisResult { * Input data for change request analysis prompt */ export interface DriftAnalysisPromptData { - /** Change request metadata */ changeRequest: ChangeRequestMetadata; - /** Component being analyzed */ component: ArchitecturalComponent; - /** Aggregated dependency changes */ + /** Aggregated dependency changes across all commits */ dependencies: DependencyExtractionResult; /** Architectural context */ architectural: { @@ -127,7 +116,7 @@ export interface DriftAnalysisPromptData { dependents: (ArchitecturalComponent & { repository?: string })[]; relationships: ComponentRelationshipRef[]; }; - /** Changed files in the PR with their status */ + /** Changed files with their status */ files?: GitDiffFile[]; /** All component IDs in the architecture model */ allComponentIds?: string[]; diff --git a/packages/core/src/pipelines/__tests__/analyze.test.ts b/packages/core/src/pipelines/__tests__/analyze.test.ts index 0662005..d8f4975 100644 --- a/packages/core/src/pipelines/__tests__/analyze.test.ts +++ b/packages/core/src/pipelines/__tests__/analyze.test.ts @@ -305,18 +305,6 @@ describe('runAnalyze', () => { ); }); - it('extracts dependencies using the full diff', async () => { - const prData = makePrData(); - mockFetchChangeRequest.mockResolvedValue(prData); - - await runAnalyze(makeOptions()); - - expect(mockExtractDependencies).toHaveBeenCalledOnce(); - const callArgs = mockExtractDependencies.mock.calls[0]?.[0] as { diff: string } | undefined; - expect(callArgs).toBeDefined(); - expect(typeof callArgs?.diff).toBe('string'); - }); - it('runs drift analysis with the correct component and extracted deps', async () => { const component = makeComponent('comp.api', 'API Service'); mockFindAllComponentsByRepository.mockReturnValue([component]); diff --git a/packages/core/src/pipelines/__tests__/components.test.ts b/packages/core/src/pipelines/__tests__/components.test.ts index efab92e..6fb82d1 100644 --- a/packages/core/src/pipelines/__tests__/components.test.ts +++ b/packages/core/src/pipelines/__tests__/components.test.ts @@ -27,7 +27,6 @@ vi.mock('../../utils/config.js', () => ({ // ── Imports (after mocks) ───────────────────────────────────────────────── import { runComponents } from '../components.js'; -import { createAdapter } from '../../adapters/adapter-factory.js'; describe('runComponents', () => { beforeEach(() => { @@ -45,28 +44,4 @@ describe('runComponents', () => { expect(result).toEqual(fakeComponents); }); - - it('calls validatePath with modelPath and "directory"', async () => { - mockLoadAndListComponents.mockResolvedValue([]); - - await runComponents({ modelPath: '/some/path' }); - - expect(mockValidatePath).toHaveBeenCalledWith('/some/path', 'directory'); - }); - - it('passes modelFormat to createAdapter', async () => { - mockLoadAndListComponents.mockResolvedValue([]); - - await runComponents({ modelPath: '/models/arch', modelFormat: 'structurizr' }); - - expect(createAdapter).toHaveBeenCalledWith('structurizr'); - }); - - it('passes modelPath to adapter.loadAndListComponents', async () => { - mockLoadAndListComponents.mockResolvedValue([]); - - await runComponents({ modelPath: '/models/arch' }); - - expect(mockLoadAndListComponents).toHaveBeenCalledWith('/models/arch'); - }); }); diff --git a/packages/core/src/pipelines/__tests__/connections.test.ts b/packages/core/src/pipelines/__tests__/connections.test.ts index a1c335c..9fd1439 100644 --- a/packages/core/src/pipelines/__tests__/connections.test.ts +++ b/packages/core/src/pipelines/__tests__/connections.test.ts @@ -52,10 +52,7 @@ import type { ArchitecturalComponent } from '../../adapters/architecture-types.j // ── Test data builders ──────────────────────────────────────────────────── -function makeComponent( - id = 'comp.api', - name = 'API Service' -): ArchitecturalComponent { +function makeComponent(id = 'comp.api', name = 'API Service'): ArchitecturalComponent { return { id, name, @@ -65,7 +62,9 @@ function makeComponent( }; } -function makeOptions(overrides: Partial<{ modelPath: string; modelFormat: string; repo: string }> = {}) { +function makeOptions( + overrides: Partial<{ modelPath: string; modelFormat: string; repo: string }> = {} +) { return { modelPath: '/path/to/model', repo: 'org/repo', @@ -166,24 +165,6 @@ describe('runConnections', () => { const results = await runConnections(makeOptions(), mockProgress); expect(results).toEqual([]); - expect(mockProgress.warn).toHaveBeenCalledWith( - 'No components found for repository: org/repo' - ); - }); - - it('passes modelFormat to createAdapter', async () => { - mockFindAllComponentsByRepository.mockReturnValue([]); - - await runConnections(makeOptions({ modelFormat: 'structurizr' })); - - expect(mockCreateAdapter).toHaveBeenCalledWith('structurizr'); - }); - - it('calls validatePath with modelPath and "directory"', async () => { - mockFindAllComponentsByRepository.mockReturnValue([]); - - await runConnections(makeOptions({ modelPath: '/custom/model/path' })); - - expect(mockValidatePath).toHaveBeenCalledWith('/custom/model/path', 'directory'); + expect(mockProgress.warn).toHaveBeenCalledWith('No components found for repository: org/repo'); }); }); diff --git a/packages/core/src/pipelines/__tests__/pr-creation.test.ts b/packages/core/src/pipelines/__tests__/pr-creation.test.ts index 8ae3318..24ffaf5 100644 --- a/packages/core/src/pipelines/__tests__/pr-creation.test.ts +++ b/packages/core/src/pipelines/__tests__/pr-creation.test.ts @@ -179,16 +179,4 @@ describe('closeModelPr', () => { expect(mockCloseChangeRequest).toHaveBeenCalledWith('erode/org-repo/pr-42'); }); - - it('constructs the branch name using modelPrBranchName', async () => { - await closeModelPr({ - repositoryUrl: 'https://github.com/org/repo', - owner: 'org', - repo: 'model-repo', - sourceRepo: 'org/repo', - prNumber: 100, - }); - - expect(mockCloseChangeRequest).toHaveBeenCalledWith('erode/org-repo/pr-100'); - }); }); diff --git a/packages/core/src/platforms/__tests__/platform-factory.test.ts b/packages/core/src/platforms/__tests__/platform-factory.test.ts index a2658c1..6014921 100644 --- a/packages/core/src/platforms/__tests__/platform-factory.test.ts +++ b/packages/core/src/platforms/__tests__/platform-factory.test.ts @@ -80,11 +80,6 @@ describe('createPlatformReader', () => { expect(reader).toBeInstanceOf(GitHubReader); }); - it('should pass token to GitHubReader', () => { - const reader = createPlatformReader('https://github.com/org/repo/pull/42', 'custom-token'); - expect(reader).toBeInstanceOf(GitHubReader); - }); - it('should return a GitLabReader for GitLab URLs', () => { const reader = createPlatformReader('https://gitlab.com/org/repo/-/merge_requests/1'); expect(reader).toBeInstanceOf(GitLabReader); diff --git a/packages/core/src/platforms/__tests__/reader-contract.ts b/packages/core/src/platforms/__tests__/reader-contract.ts index 2343514..e1658c9 100644 --- a/packages/core/src/platforms/__tests__/reader-contract.ts +++ b/packages/core/src/platforms/__tests__/reader-contract.ts @@ -16,36 +16,11 @@ export function runReaderContractTests( ): void { describe('SourcePlatformReader contract', () => { describe('parseChangeRequestUrl', () => { - it('should return a ChangeRequestRef with required fields for a valid URL', () => { - const reader = getReader(); - const ref = reader.parseChangeRequestUrl(fixtures.validUrl); - - expect(ref).toBeDefined(); - expect(typeof ref.number).toBe('number'); - expect(ref.number).toBeGreaterThan(0); - expect(typeof ref.url).toBe('string'); - expect(ref.url.length).toBeGreaterThan(0); - expect(typeof ref.repositoryUrl).toBe('string'); - expect(ref.repositoryUrl.length).toBeGreaterThan(0); - expect(ref.platformId).toBeDefined(); - expect(typeof ref.platformId).toBe('object'); - }); - it('should throw for an invalid URL', () => { const reader = getReader(); expect(() => reader.parseChangeRequestUrl(fixtures.invalidUrl)).toThrow(); }); - it('should return consistent results for the same URL', () => { - const reader = getReader(); - const ref1 = reader.parseChangeRequestUrl(fixtures.validUrl); - const ref2 = reader.parseChangeRequestUrl(fixtures.validUrl); - - expect(ref1.number).toBe(ref2.number); - expect(ref1.repositoryUrl).toBe(ref2.repositoryUrl); - expect(ref1.url).toBe(ref2.url); - }); - it('should preserve the original URL in the ref', () => { const reader = getReader(); const ref = reader.parseChangeRequestUrl(fixtures.validUrl); @@ -59,22 +34,5 @@ export function runReaderContractTests( expect(fixtures.validUrl.startsWith(ref.repositoryUrl)).toBe(true); }); }); - - describe('interface shape', () => { - it('should implement fetchChangeRequest as a function', () => { - const reader = getReader(); - expect(typeof reader.fetchChangeRequest).toBe('function'); - }); - - it('should implement fetchChangeRequestCommits as a function', () => { - const reader = getReader(); - expect(typeof reader.fetchChangeRequestCommits).toBe('function'); - }); - - it('should implement parseChangeRequestUrl as a function', () => { - const reader = getReader(); - expect(typeof reader.parseChangeRequestUrl).toBe('function'); - }); - }); }); } diff --git a/packages/core/src/platforms/bitbucket/__tests__/api-client.test.ts b/packages/core/src/platforms/bitbucket/__tests__/api-client.test.ts index 8040668..31d6e39 100644 --- a/packages/core/src/platforms/bitbucket/__tests__/api-client.test.ts +++ b/packages/core/src/platforms/bitbucket/__tests__/api-client.test.ts @@ -173,9 +173,7 @@ describe('BitbucketApiClient', () => { describe('requestText', () => { it('should return text on success', async () => { - vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce( - mockFetchResponse('diff content here') - ); + vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce(mockFetchResponse('diff content here')); const result = await client.requestText('/repos/org/repo/diff'); @@ -199,15 +197,13 @@ describe('BitbucketApiClient', () => { describe('requestVoid', () => { it('should resolve void on success', async () => { - vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce( - mockFetchResponse('', true, 204) - ); + vi.spyOn(globalThis, 'fetch').mockResolvedValueOnce(mockFetchResponse('', true, 204)); - const result = await client.requestVoid('/repos/org/repo/approve', { + await client.requestVoid('/repos/org/repo/approve', { method: 'POST', }); - expect(result).toBeUndefined(); + // requestVoid returns void on success (204) }); it('should throw ApiError on non-ok response', async () => { diff --git a/packages/core/src/platforms/bitbucket/__tests__/reader.test.ts b/packages/core/src/platforms/bitbucket/__tests__/reader.test.ts index 0850f6d..5c65147 100644 --- a/packages/core/src/platforms/bitbucket/__tests__/reader.test.ts +++ b/packages/core/src/platforms/bitbucket/__tests__/reader.test.ts @@ -119,13 +119,6 @@ describe('BitbucketReader', () => { it('should throw ErodeError for empty string', () => { expect(() => reader.parseChangeRequestUrl('')).toThrow(ErodeError); }); - - it('should return consistent results for the same URL', () => { - const url = 'https://bitbucket.org/workspace/repo/pull-requests/42'; - const ref1 = reader.parseChangeRequestUrl(url); - const ref2 = reader.parseChangeRequestUrl(url); - expect(ref1).toEqual(ref2); - }); }); describe('fetchChangeRequest', () => { diff --git a/packages/core/src/platforms/bitbucket/writer.ts b/packages/core/src/platforms/bitbucket/writer.ts index 7e5efe5..31abdc9 100644 --- a/packages/core/src/platforms/bitbucket/writer.ts +++ b/packages/core/src/platforms/bitbucket/writer.ts @@ -53,7 +53,6 @@ export class BitbucketWriter implements SourcePlatformWriter { // Bitbucket has no native draft PRs — the draft option is ignored. const repoPath = `/repositories/${this.workspace}/${this.repoSlug}`; - // Check if branch exists let branchExists = false; try { await this.api.requestVoid(`${repoPath}/refs/branches/${encodeURIComponent(branchName)}`); @@ -63,7 +62,6 @@ export class BitbucketWriter implements SourcePlatformWriter { } if (!branchExists) { - // Get the base branch hash to use as the new branch target const baseBranchInfo = await this.api.request( `${repoPath}/refs/branches/${encodeURIComponent(baseBranch)}`, z.object({ target: z.object({ hash: z.string() }) }).loose() @@ -78,7 +76,6 @@ export class BitbucketWriter implements SourcePlatformWriter { }); } - // Commit files via the source endpoint (multipart form data) if (fileChanges.length > 0) { const formData = new FormData(); formData.append('branch', branchName); @@ -92,7 +89,6 @@ export class BitbucketWriter implements SourcePlatformWriter { }); } - // Check for an existing open PR from this branch const existingPrs = await this.api.request( `${repoPath}/pullrequests?q=${encodeURIComponent(`source.branch.name="${branchName}" AND state="OPEN"`)}`, BitbucketPrListResponseSchema @@ -117,7 +113,6 @@ export class BitbucketWriter implements SourcePlatformWriter { }; } - // Create a new PR const pr = await this.api.request( `${repoPath}/pullrequests`, BitbucketPrWriteResponseSchema, diff --git a/packages/core/src/platforms/gitlab/__tests__/reader.test.ts b/packages/core/src/platforms/gitlab/__tests__/reader.test.ts index d2ea1bd..29dd5b9 100644 --- a/packages/core/src/platforms/gitlab/__tests__/reader.test.ts +++ b/packages/core/src/platforms/gitlab/__tests__/reader.test.ts @@ -135,13 +135,6 @@ describe('GitLabReader', () => { it('should throw ErodeError for empty string', () => { expect(() => reader.parseChangeRequestUrl('')).toThrow(ErodeError); }); - - it('should return consistent results for the same URL', () => { - const url = 'https://gitlab.com/org/repo/-/merge_requests/42'; - const ref1 = reader.parseChangeRequestUrl(url); - const ref2 = reader.parseChangeRequestUrl(url); - expect(ref1).toEqual(ref2); - }); }); describe('fetchChangeRequest', () => { diff --git a/packages/core/src/providers/anthropic/__tests__/provider.test.ts b/packages/core/src/providers/anthropic/__tests__/provider.test.ts index a081391..62c91d7 100644 --- a/packages/core/src/providers/anthropic/__tests__/provider.test.ts +++ b/packages/core/src/providers/anthropic/__tests__/provider.test.ts @@ -104,11 +104,6 @@ describe('AnthropicProvider', () => { it('should throw on missing API key', () => { expect(() => new AnthropicProvider({ apiKey: '' })).toThrow(ErodeError); }); - - it('should create provider with valid API key', () => { - const provider = createProvider(); - expect(provider).toBeDefined(); - }); }); describe('selectComponent', () => { diff --git a/packages/core/src/providers/anthropic/provider.ts b/packages/core/src/providers/anthropic/provider.ts index 5d411a8..d958a7e 100644 --- a/packages/core/src/providers/anthropic/provider.ts +++ b/packages/core/src/providers/anthropic/provider.ts @@ -57,7 +57,6 @@ export class AnthropicProvider extends BaseProvider { ); } - // Check for truncation if (response.stop_reason === 'max_tokens') { throw new ErodeError( 'Anthropic response was cut short (max_tokens reached)', diff --git a/packages/core/src/providers/gemini/__tests__/provider.test.ts b/packages/core/src/providers/gemini/__tests__/provider.test.ts index 3114385..53b76e1 100644 --- a/packages/core/src/providers/gemini/__tests__/provider.test.ts +++ b/packages/core/src/providers/gemini/__tests__/provider.test.ts @@ -106,11 +106,6 @@ describe('GeminiProvider', () => { it('should throw on missing API key', () => { expect(() => new GeminiProvider({ apiKey: '' })).toThrow(ErodeError); }); - - it('should create provider with valid API key', () => { - const provider = createProvider(); - expect(provider).toBeDefined(); - }); }); describe('selectComponent', () => { diff --git a/packages/core/src/providers/openai/__tests__/provider.test.ts b/packages/core/src/providers/openai/__tests__/provider.test.ts index 8af28d8..863f242 100644 --- a/packages/core/src/providers/openai/__tests__/provider.test.ts +++ b/packages/core/src/providers/openai/__tests__/provider.test.ts @@ -97,11 +97,6 @@ describe('OpenAIProvider', () => { it('should throw on missing API key', () => { expect(() => new OpenAIProvider({ apiKey: '' })).toThrow(ErodeError); }); - - it('should create provider with valid API key', () => { - const provider = createProvider(); - expect(provider).toBeDefined(); - }); }); describe('selectComponent', () => { diff --git a/packages/core/src/utils/__tests__/config-file.test.ts b/packages/core/src/utils/__tests__/config-file.test.ts index 31b59d7..f1231c7 100644 --- a/packages/core/src/utils/__tests__/config-file.test.ts +++ b/packages/core/src/utils/__tests__/config-file.test.ts @@ -10,14 +10,7 @@ vi.mock('fs', () => ({ })); import * as fs from 'fs'; -import { - findConfigFile, - RC_FILENAME, - ENV_VAR_NAMES, - deepMerge, - loadConfigFromFile, - loadConfigFromEnv, -} from '../config.js'; +import { findConfigFile, deepMerge, loadConfigFromFile, loadConfigFromEnv } from '../config.js'; import { ConfigurationError } from '../../errors.js'; const mockExistsSync = vi.mocked(fs.existsSync); @@ -29,12 +22,6 @@ describe('config file support', () => { mockReadFileSync.mockReset(); }); - describe('RC_FILENAME', () => { - it('should be .eroderc.json', () => { - expect(RC_FILENAME).toBe('.eroderc.json'); - }); - }); - describe('findConfigFile', () => { it('should return cwd path when config file exists there', () => { const cwdPath = path.join(process.cwd(), '.eroderc.json'); @@ -89,33 +76,6 @@ describe('config file support', () => { }); }); - describe('ENV_VAR_NAMES', () => { - it('should have all expected keys', () => { - expect(ENV_VAR_NAMES).toHaveProperty('aiProvider'); - expect(ENV_VAR_NAMES).toHaveProperty('geminiApiKey'); - expect(ENV_VAR_NAMES).toHaveProperty('anthropicApiKey'); - expect(ENV_VAR_NAMES).toHaveProperty('openaiApiKey'); - expect(ENV_VAR_NAMES).toHaveProperty('githubToken'); - expect(ENV_VAR_NAMES).toHaveProperty('gitlabToken'); - expect(ENV_VAR_NAMES).toHaveProperty('bitbucketToken'); - expect(ENV_VAR_NAMES).toHaveProperty('structurizrCliPath'); - expect(ENV_VAR_NAMES).toHaveProperty('modelRepoPrToken'); - expect(ENV_VAR_NAMES).toHaveProperty('modelPath'); - expect(ENV_VAR_NAMES).toHaveProperty('modelRepo'); - expect(ENV_VAR_NAMES).toHaveProperty('modelRef'); - }); - - it('should have all values prefixed with ERODE_', () => { - for (const value of Object.values(ENV_VAR_NAMES)) { - expect(value).toMatch(/^ERODE_/); - } - }); - - it.each(Object.entries(ENV_VAR_NAMES))('should map %s to %s', (key, value) => { - expect(ENV_VAR_NAMES[key as keyof typeof ENV_VAR_NAMES]).toBe(value); - }); - }); - describe('deepMerge', () => { it('should override target values with source values', () => { const target = { ai: { provider: 'gemini' } }; diff --git a/packages/core/src/utils/__tests__/retry.test.ts b/packages/core/src/utils/__tests__/retry.test.ts index 19f56d6..4ecc52a 100644 --- a/packages/core/src/utils/__tests__/retry.test.ts +++ b/packages/core/src/utils/__tests__/retry.test.ts @@ -64,13 +64,7 @@ describe('withRetry', () => { }); it('throws immediately for non-recoverable ErodeError without custom shouldRetry', async () => { - const error = new ErodeError( - 'fail', - ErrorCode.CONFIG_INVALID, - undefined, - {}, - false - ); + const error = new ErodeError('fail', ErrorCode.CONFIG_INVALID, undefined, {}, false); const operation = vi.fn().mockRejectedValue(error); await expect(withRetry(operation, FAST_POLICY)).rejects.toThrow(error); @@ -80,9 +74,9 @@ describe('withRetry', () => { it('throws last error after exhausting all retries', async () => { const operation = vi.fn().mockRejectedValue(new Error('always fails')); - await expect( - withRetry(operation, { ...FAST_POLICY, retries: 1 }) - ).rejects.toThrow('always fails'); + await expect(withRetry(operation, { ...FAST_POLICY, retries: 1 })).rejects.toThrow( + 'always fails' + ); // 1 initial attempt + 1 retry = 2 calls expect(operation).toHaveBeenCalledTimes(2); @@ -90,26 +84,24 @@ describe('withRetry', () => { it('logs retry message when CONFIG.debug.verbose is true', async () => { mockConfig.debug.verbose = true; - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - const operation = vi - .fn() - .mockRejectedValueOnce(new Error('transient')) - .mockResolvedValue('ok'); + const consoleSpy = vi + .spyOn(console, 'error') + // eslint-disable-next-line @typescript-eslint/no-empty-function + .mockImplementation(() => {}); + const operation = vi.fn().mockRejectedValueOnce(new Error('transient')).mockResolvedValue('ok'); await withRetry(operation, FAST_POLICY); - expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining('[Retry]') - ); + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('[Retry]')); }); it('does not log when verbose is false', async () => { mockConfig.debug.verbose = false; - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - const operation = vi - .fn() - .mockRejectedValueOnce(new Error('transient')) - .mockResolvedValue('ok'); + const consoleSpy = vi + .spyOn(console, 'error') + // eslint-disable-next-line @typescript-eslint/no-empty-function + .mockImplementation(() => {}); + const operation = vi.fn().mockRejectedValueOnce(new Error('transient')).mockResolvedValue('ok'); await withRetry(operation, FAST_POLICY); diff --git a/packages/core/src/utils/validation.ts b/packages/core/src/utils/validation.ts index 9e7e85e..ad6a437 100644 --- a/packages/core/src/utils/validation.ts +++ b/packages/core/src/utils/validation.ts @@ -24,7 +24,6 @@ export function validatePath(path: string, type: 'file' | 'directory' = 'directo ); } - // Verify the path matches the expected type const stats = statSync(path); if (type === 'file' && !stats.isFile()) { throw new ErodeError( From 6e8d42e39128463cecca0fe3f39982333ab70ac5 Mon Sep 17 00:00:00 2001 From: Anders Hassis Date: Sun, 8 Mar 2026 14:43:30 +0100 Subject: [PATCH 4/4] fix(core): use rejects.toBe for exact error instance assertion in retry test --- packages/core/src/utils/__tests__/retry.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/utils/__tests__/retry.test.ts b/packages/core/src/utils/__tests__/retry.test.ts index 4ecc52a..0f28546 100644 --- a/packages/core/src/utils/__tests__/retry.test.ts +++ b/packages/core/src/utils/__tests__/retry.test.ts @@ -67,7 +67,7 @@ describe('withRetry', () => { const error = new ErodeError('fail', ErrorCode.CONFIG_INVALID, undefined, {}, false); const operation = vi.fn().mockRejectedValue(error); - await expect(withRetry(operation, FAST_POLICY)).rejects.toThrow(error); + await expect(withRetry(operation, FAST_POLICY)).rejects.toBe(error); expect(operation).toHaveBeenCalledTimes(1); });