chore: test discipline + DX upgrades - #18
Closed
ChS23 wants to merge 32 commits into
Closed
Conversation
A foundational test-quality upgrade aimed at the enterprise architects the tool targets. Existing line/statement coverage is already excellent (96.5%/97.5%); the gap is in branches (88.9%) where mutation-survivable bugs hide — five such bugs shipped to 2.1.4/2.1.5 and they all fell in that gap. Tooling added - @vitest/coverage-v8 with threshold floors (95/85/98/95) set just below current numbers so CI catches regressions without blocking routine work. - @fast-check/vitest for property-based testing. - @stryker-mutator/core + @stryker-mutator/vitest-runner for mutation testing; config focused on src/rules and src/generators where the recurring bug class lives. Not wired to CI yet — opt-in via `pnpm test:mutation`. - execa for E2E CLI subprocess testing. - Vitest bumped to 4.1.6 to align with coverage peer dep and unlock Test Tags / projects features. Test layout - vitest projects split: unit (test/), integration (examples/), e2e (test/e2e/). Each with its own timeout. Run individually via `pnpm test:unit`, `test:integration`, `test:e2e`. - test/property/rules-options.test.ts — 11 property tests covering every option-bearing rule (acl, crud, dbPerService, stableDependencies, apiGateway). Each test asserts the rule respects a randomly-generated option value, defending against the "literal-instead-of-option" bug class. - test/property/fix-invariants.test.ts — 7 invariant tests for fix functions (never throws, deterministic, produces edits, round-trips through applyEdits). - test/e2e/cli.test.ts — 9 subprocess tests that invoke dist/cli/index.mjs as a real binary in a fresh tmp directory. Covers init/check/--fix loop, friendly errors, --help, exit codes — the parts unit tests cannot exercise. CI - test.yaml now runs coverage on every Node version, builds the CLI, and runs e2e separately. Coverage report uploaded as artifact on Node 22. Result - 290 → 317 tests (+27). - Branch coverage 88.9% → 89.26% (property tests opened a few more paths, more to come). - Pre-merge confidence covers a class of bugs that example-based tests fundamentally cannot reach.
- move @fast-check/vitest property tests from test/property/ into the per-source <rule>.test.ts file each one belongs to (acl, crud, dbPerService, stableDependencies, apiGateway, fixAcl, fixCrud, fixDbPerService), matching the established test architecture; drop the now-empty test/property/ directory and its eslint override - add inline regression snapshots for the kubernetes and plantumlFromModel generators so silent output drifts surface in the diff - add syntax-helper tests for plantumlSyntax and structurizrDslSyntax (containerDecl/relationDecl with and without tags), closing the previously uncovered "no-tags" and "with-tags" branches - close gap branches: apiGateway relation without technology field; boundaryUtils publicApi === owner fallback path (non-tagged fallback owner case); commonReuse container with no enclosing boundary; applyEdits multi-line content with blank lines and ambiguous-search warning; fix dbPerService multiple tagged owners warning - c8 ignore truly unreachable code paths (TypeScript exhaustive `: never` guard in loadModel, defensive `return null` in check.getSyntax, regex `?? ""` narrowing fallback in fix.applyEdits, empty-synonymes-map loop in kubernetes mapContainersFromDeployConfigs) — each with a comment explaining why and when to remove the ignore - bump coverage thresholds in vitest.config.ts from 95/85/98/95 to 97/90/99/98 to lock in the achieved floor Result: 324 tests across unit/integration/e2e, coverage 97.32% stmts / 90.82% branches / 99.57% funcs / 98.36% lines.
…ertions Stryker baseline run (1007 mutants, 18 files) showed two systemic gaps: 1. registry.ts had 0% mutation score — the canonical rules registry was never asserted, so renames, missing fixes, or wrong wiring slipped silently past tests. 2. Many tests checked `violations.length` but not `violations[0].message`, so StringLiteral mutations on user-facing messages survived. Changes: - test/rules/registry.test.ts (new): asserts the registry holds exactly the eight published rules with unique names, that fix is exposed only for acl/crud/dbPerService, that each check entry routes to the underlying check function, and that each fix entry routes to its matching fixer. Kills all 26 registry mutants. - test/rules/acl.test.ts: pin "system" vs "systems" pluralization and the "without an ACL layer" suffix. - test/rules/crud.test.ts: pin full violation messages by-value (DB name, repo non-db deps). - test/rules/cohesion.test.ts: pin coupling-vs-cohesion message format and add boundary case where cohesion strictly exceeds coupling; pin parent-vs-inner message in a dedicated model. - test/rules/stableDependencies.test.ts: pin message regex; add the equal-instability boundary (strict `<` not `<=`); add a 3-node cycle to guard counter increments. - test/rules/fix.test.ts: spy on consola.warn and assert the warning text for pattern-not-found and ambiguous-pattern cases; add a no-warn boundary for matchCount=1. - test/rules/boundaryUtils.test.ts: assert warn text for both no-public-API and only-candidate-is-owner branches; add a tie-break smoke test for equal in-degrees. Stryker config: - vitest.mutation.config.ts (new): slimmed-down vitest config exposing only the unit suite, since @stryker-mutator/vitest-runner v9 has no per-project filter for the main config. - stryker.config.mjs: add explicit `plugins` entry (pnpm's flat-symlink layout breaks auto-discovery), point at the dedicated config, enable `coverageAnalysis: "all"` and add a JSON reporter for downstream scripting. Document why ignoreStatic was removed. Pre-tightening baseline score 78.75 (registry at 0); post-registry-only run 81.73 (registry at 100). Further improvements queued for next run.
Second wave of targeted tests aimed at Stryker survivors:
- fixDbPerService.test.ts: spy on consola.warn for both no-tagged and
multi-tagged paths; pin a boundary case where one tagged accessor
must NOT trigger the multi-tagged warning; assert name+type lookup
is conjunctive; assert empty technology produces `""` rendering;
assert single-accessor case yields no fix.
- fixCrud.test.ts: spy on warn for repo-already-exists; assert FixResult
carries rule="crud" and a meaningful description; pin the derived
label format ("Payment processor Repo") so StringLiteral mutations
on the construction expression don't survive.
- fixAcl.test.ts: spy on warn for already-exists path; assert silent
skip for non-existent container; assert no fix when violation has
no external relations; pin edits.length=3 to kill the ArrayDeclaration
initial-value mutation.
- stableDependencies.test.ts: pin the isolated-container 1.0-instability
path; assert external→internal relations do not inflate Ca; pin the
externalType option propagation through both filter branches.
Mutation score trajectory: 78.75 (baseline) → 81.73 (registry) → 85.00
(rules messages + warn) → 86.69 (fixCrud + fixDbPerService — this run).
The next run will include the fixAcl + stableDependencies additions
above and should push past 88%.
- fix.ts: drop defensive optional+nullish around regex match - fixDbPerService: drop defensive rel-not-found bail - fixAcl: pin container lookup by exact name - fixCrud: pin .filter, c !== accessor, .some, custom repoTags - fixDbPerService: pin .some-vs-.every, missing-db, accessor .some Mutation score: 87.29 -> 89.30. fixAcl now 100%.
chore(stryker): extend scope to loaders so model load/parse mutants count
- fixCrud: remove redundant dbRels.length === 0 early return; Stryker- disable equivalent c !== accessor check; new tests for .some-vs-.every and no-boundary accessor/db paths. - fixDbPerService: Stryker-disable equivalent accessors.length <= 1 and name+type LogicalOperator; new test for zero-accessor non-throwing. - plantumlFromModel: inline snapshot of full project-boundary output; empty-tags-array tests for container and relation. - plantuml (legacy): full multi-config inline snapshot; transport+async pin tests; kafka-fanout some-vs-every; ext-system dedup; closing brace.
- dslId: identifier property vs raw id fallback - isDatabase: technology substring matches (postgresql, mysql, redis, mongodb) and name suffix matches (_db, database) - enrichTags: comma split + trim, repo/acl auto-tag, dedup, empty filter - addRelations: technology preservation, description fallback rule, async tag composition, missing destination skip, component-level recursion - mapContainersFromStructurizr: external by location, people with Person type, alphabetic sort
plantuml: e2e tests via crafted PUML fixtures pin every container type, the Rel_Back swap, tags split, technology parse, sort, boundary nesting. kubernetes: tempdir fixture tests pin .yml/.yaml ext filter, default exclude list, microservice envelope unwrap, env: -> environment, all cleanup substrings, prod-vs-default fallback, lowercase, fileName fallback, name normalisation, sort, custom whitelist + cleanup options. eslint: relax sonarjs/no-clear-text-protocols, no-alphabetical-sort, no-identical-functions, unicorn/import-style for test/** — these are fixture-shape rules irrelevant to test code.
structurizr loader: description fallback, external system tag split, people-relationships loop, components loop non-throw. plantuml loader: System and Person type recognition; silent skip on Rel() with unknown endpoints. fixDbPerService multi-tagged warning: switched from vi.spyOn to direct consola.warn assignment — vi.spyOn was not propagating through Stryker's worker process for some reason; manual assignment is portable.
boundaryUtils: stryker-disable equivalent early returns and inDegree init; tests for in-degree calc excluding same-boundary edges and the sort comparator nullish coalescing. namingUtils: stryker-disable empty-names early return; tests for the strict > vs >= boundary, && vs || logical, and snake-only no-trigger.
structurizr: tests for fallback array iteration (containers/people/ softwareSystems undefined), the conditional async detection, .filter(Boolean) on tags, .map(trim) inside relation tags. cohesion: pin counting external rels on inner-boundary containers into parent coupling; pin strict >= boundary for parent-vs-inner check.
… dbPerService, namingUtils Pin exact violation messages on commonReuse, acyclic, dbPerService. Add pubNames.size < 2 boundary for commonReuse and visited.has skip guard for acyclic. apiGateway with undefined technology. namingUtils hyphen-vs-camel strict > boundary.
stableDependencies: equivalent counter arithmetic, internal-name guard, and isolated-instability early return. structurizr: ?? "" fallbacks on toLowerCase and `?? []` on workspace iteration arrays. kubernetes defaults: per-string mutations covered by integration test surface, not worth pinning. plantuml legacy generator internal dedup arrays.
Static badges are dishonest signal (numbers go stale). Wiring up Codecov + Stryker Dashboard for live badges needs maintainer to enrol the upstream repo and add CI secrets — out of contributor scope. Leave the call to upstream.
Bumped 8 packages — patch/minor only, no risk: - valibot 1.2 -> 1.4 - yaml 2.8 -> 2.9 - citty 0.2.0 -> 0.2.2 (UnJS) - jiti 2.6 -> 2.7 (UnJS) - prettier 3.8.1 -> 3.8.3 - prettier-plugin-packagejson 3.0.0 -> 3.0.2 - globals 17.3 -> 17.6 - typescript-eslint 8.54 -> 8.59 487 tests still pass; lint green. Held back (require dedicated PR): typescript 5.9->6.0 (transitional to native Go port in 7.0 - needs full regression on tsc semantics, strict mode default, module interop), eslint 9->10 (flat config + plugins migration), commitlint 20->21, eslint-plugin-* majors, @types/node 25 (blocked by Node 20 engine support). README: added Testing section documenting the four test tiers, coverage thresholds, and mutation score targets — surfaces the discipline we built.
Setup: - changelogen 0.6.2 as devDep - changelog.config.ts: types in display order (feat/perf/fix/refactor/ docs/test/build/chore), no emoji on section headers, hideAuthorEmail for public-safe contributor lists, Byndyusoft/aact repo for compare + issue + PR links - package.json scripts: `pnpm changelog` (preview) and `pnpm release` (--release --push: bump version, generate CHANGELOG, commit, tag, push) - CHANGELOG.md skeleton — content generated on first release Cleanup from typescript-eslint 8.59 stricter detection: - removed 19 unused eslint-disable directives across test/generators/ and other files (auto-fix) - removed 2 unnecessary type assertions in tests - added eslint override: n/no-extraneous-import off (config files legitimately import devDeps for typing)
Switched src/cli/commands/check.ts from picocolors to consola/utils
`colors` — identical API surface, one fewer runtime dep, fully aligned
with UnJS stack.
Final `aact check` summary now uses `consola/utils.box` instead of a
plain line:
- green box with ✓ title when no violations
- red box with ✗ title when violations exist
- dim hint line below count showing how many rules have auto-fix
available, e.g. "2 rules have auto-fix — run with --fix"
Saves one dep (`picocolors`) and gives a noticeably more polished
final verdict without adding boxen/figlet etc.
knip и publint в скриптах: `pnpm knip`, `pnpm publint` — обе зелёные. eslint plugins (cherry-picked rules, без full presets): - eslint-plugin-citty в src/cli - eslint-plugin-import-x для no-cycle + type-imports - @vitest/eslint-plugin (новый официальный) - @eslint-community/eslint-plugin-eslint-comments — no-unused-disable Auto-fix почистил импорты в 27 местах. Два informational теста в banking-plantuml получили toBeDefined() чтобы плагин не ругался.
Node 22 — Maintenance LTS (до 2027-04), Node 24 — Active LTS. @types/node до ^22, CI matrix: 22.x + 24.x.
Normalizes path separators to POSIX on Windows (мисматчи \\ vs / в path matching ловятся как тонкие баги). Drop-in API replacement, 7 файлов, simple-import-sort auto-fix переставил импорты.
9 tasks
Member
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d68d7404b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| extends: true, | ||
| test: { | ||
| name: "e2e", | ||
| include: ["test/e2e/**/*.test.ts"], |
There was a problem hiding this comment.
Including the e2e project in the default Vitest project list makes pnpm test fail on a clean checkout because the new e2e suite hard-errors when dist/cli/index.mjs is missing (test/e2e/cli.test.ts beforeAll throws unless pnpm build was run first). This turns the default test command into a build-order-dependent workflow and breaks common local/CI usage that expects tests to run without a prior manual build; either prebuild in the test script or keep e2e opt-in (test:e2e).
Useful? React with 👍 / 👎.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Фундамент для дальнейших рефакторингов — mutation-grade safety net и обновлённый dev tooling. Behavior не меняется, public API не тронут.
Тесты
projectsconfig — разделены unit/integration/e2e suitesTooling / DX
consola/utilscolors + box вместо picocolors — одна dep меньше, чистый UnJSpatheвместоnode:path— Windows path normalizationchangelogenдля автоматического CHANGELOGknip,publint, дополнительные ESLint plugins (citty, import-x, @vitest/eslint-plugin, eslint-comments)