From 8976ea7c3d80cca4c8745ed5aff40f611dfe30eb Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Thu, 14 May 2026 20:35:23 +0300 Subject: [PATCH] chore: enforce test structure rules --- AGENTS.md | 2 +- scripts/steiger-rules.ts | 90 +++++++++++++++++++++- steiger.config.ts | 6 +- tests/engine/scripts/steiger-rules.test.ts | 29 +++++++ 4 files changed, 122 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 701f598ce..ef430e0ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -239,7 +239,7 @@ Release commits are the exception: keep using `Release v0.x.y`. - Do not place code or tests ad hoc. Before adding or moving files, inspect the existing folder structure and nearby patterns, then put changes in the established domain-specific location. If no proper location exists, create one deliberately and update docs/conventions as needed. - Architecture boundaries are enforced by Steiger (`bun run check:arch`). App code must use public workspace package exports, workspace packages must not import app `src/` code, package-local aliases (`#core`, `#vue`, `#cli`, `#mcp`) are only for their owning package, core must stay framework-agnostic, app service/domain code (`src/app/**`) must not import app component/view layers, components must not import views, shared UI (`src/components/ui/**`) must not import app services/stores, property-panel internals must stay inside the property panel, and canvas/editor overlay code must not import property-panel internals. -- Test placement is strict: E2E tests live under `tests/e2e/**` and use `*.spec.ts`; engine/unit tests live under `tests/engine/**` and use `*.test.ts`. Do not put store-only/internal-state assertions in E2E. If a test drives the UI like a user and verifies visible behavior, it can be E2E; if it creates nodes through internals and asserts graph state, it belongs in engine/unit coverage. +- Test placement is strict and enforced by Steiger: app E2E tests live under `tests/e2e/**` and use `*.spec.ts`; Figma automation tests live under `tests/figma/**` and use `*.spec.ts`; engine/unit tests live under `tests/engine/**` and use `*.test.ts` (with `helpers.ts`, `*.bench.ts`, and `visual-*` support scripts allowed); shared test utilities live under `tests/helpers/**`. Do not put store-only/internal-state assertions in E2E. If a test drives the UI like a user and verifies visible behavior, it can be E2E; if it creates nodes through internals and asserts graph state, it belongs in engine/unit coverage. ### File and folder naming diff --git a/scripts/steiger-rules.ts b/scripts/steiger-rules.ts index f9dba3676..01c3cd508 100644 --- a/scripts/steiger-rules.ts +++ b/scripts/steiger-rules.ts @@ -15,6 +15,8 @@ type Diagnostic = { type RuleResult = { diagnostics: Diagnostic[] } type Rule = { name: string; check: (root: TreeEntry) => RuleResult } +type FileRuleCheck = (sourceRel: string) => string | null + type ImportRef = { specifier: string line: number @@ -56,8 +58,8 @@ function collectFiles(entry: TreeEntry, files: string[] = []) { function importsIn(content: string): ImportRef[] { const imports: ImportRef[] = [] const patterns = [ - /(?:import|export)\s+(?:type\s+)?(?:[^'";]*?\s+from\s*)?['"]([^'"]+)['"]/g, - /import\(\s*['"]([^'"]+)['"]\s*\)/g + /^\s*(?:import|export)\s+(?:type\s+)?(?:[^'";]*?\s+from\s*)?['"]([^'"]+)['"]/gm, + /^\s*import\(\s*['"]([^'"]+)['"]\s*\)/gm ] for (const pattern of patterns) { @@ -88,6 +90,22 @@ function resolveImport(sourceRel: string, specifier: string): string | null { return null } +function createFileRule(name: string, checkFile: FileRuleCheck): Rule { + return { + name, + check(root) { + const diagnostics: Diagnostic[] = [] + for (const file of collectFiles(root)) { + const sourceRel = relativePath(root.path, file) + const message = checkFile(sourceRel) + if (!message) continue + diagnostics.push({ message, location: { path: file } }) + } + return { diagnostics } + } + } +} + function createImportRule( name: string, checkImport: (sourceRel: string, specifier: string, resolved: string | null) => string | null @@ -114,6 +132,47 @@ function createImportRule( } } +const strictTestFilePlacement = createFileRule('open-pencil/strict-test-file-placement', (sourceRel) => { + if (!sourceRel.startsWith('tests/')) return null + if (!TEXT_EXTENSIONS.has(path.extname(sourceRel))) return null + if (sourceRel.startsWith('tests/e2e/')) { + return sourceRel.endsWith('.spec.ts') ? null : 'E2E tests must live under tests/e2e/** and use *.spec.ts.' + } + if (sourceRel.startsWith('tests/figma/')) { + return sourceRel.endsWith('.spec.ts') ? null : 'Figma Playwright tests must live under tests/figma/** and use *.spec.ts.' + } + if (sourceRel.startsWith('tests/engine/')) { + if (sourceRel.endsWith('.test.ts')) return null + if (sourceRel.endsWith('/helpers.ts') || sourceRel.endsWith('.bench.ts')) return null + if (path.basename(sourceRel).startsWith('visual-')) return null + return 'Engine/unit tests must live under tests/engine/** and use *.test.ts; helpers.ts, *.bench.ts, and visual-* support scripts are allowed.' + } + if (sourceRel.startsWith('tests/helpers/')) return null + return 'Tests must live under tests/e2e/** (*.spec.ts), tests/engine/** (*.test.ts), or tests/helpers/**.' +}) + +const noEngineOnlyAssertionsInE2E = createImportRule( + 'open-pencil/no-engine-only-assertions-in-e2e', + (sourceRel, specifier, resolved) => { + if (!sourceRel.startsWith('tests/e2e/')) return null + if (specifier === 'bun:test' || resolved?.startsWith('tests/engine/')) { + return 'E2E tests must drive the UI and visible behavior. Put engine/internal-state assertions in tests/engine/**.' + } + return null + } +) + +const noE2EImportsInEngineTests = createImportRule( + 'open-pencil/no-e2e-imports-in-engine-tests', + (sourceRel, _specifier, resolved) => { + if (!sourceRel.startsWith('tests/engine/')) return null + if (resolved?.startsWith('tests/e2e/')) { + return 'Engine/unit tests must not import E2E tests or fixtures.' + } + return null + } +) + const noPropertyPanelImportsInCanvas = createImportRule( 'open-pencil/no-property-panel-imports-in-canvas', (sourceRel, _specifier, resolved) => { @@ -158,7 +217,7 @@ const noPackageInternalsInApp = createImportRule( const noForeignPackageLocalAliases = createImportRule( 'open-pencil/no-foreign-package-local-aliases', (sourceRel, specifier) => { - if (sourceRel.startsWith('scripts/')) return null + if (sourceRel.startsWith('scripts/') || sourceRel.startsWith('tests/')) return null for (const [alias, owner] of Object.entries(PACKAGE_ALIAS_OWNERS)) { if (specifier.startsWith(alias) && !sourceRel.startsWith(owner)) { return `Package-local alias ${alias} can only be used inside ${owner}. Use a public package export across package boundaries.` @@ -192,6 +251,26 @@ const noComponentsImportViews = createImportRule( } ) +const noNonUiImportsInSharedUi = createImportRule( + 'open-pencil/no-non-ui-imports-in-shared-ui', + (sourceRel, _specifier, resolved) => { + if (!sourceRel.startsWith('src/components/ui/')) return null + if (resolved?.startsWith('src/components/') && !resolved.startsWith('src/components/ui/')) { + return 'Shared UI components must only import other shared UI modules from src/components/ui/**.' + } + return null + } +) + +const noViewsImportedOutsideEntry = createImportRule( + 'open-pencil/no-views-imported-outside-entry', + (sourceRel, _specifier, resolved) => { + if (!resolved?.startsWith('src/views/')) return null + if (sourceRel === 'src/App.vue' || sourceRel === 'src/main.ts' || sourceRel === 'src/router.ts') return null + return 'Views are top-level composition entrypoints and must not be imported by app services or reusable components.' + } +) + const noAppImportsInSharedUi = createImportRule( 'open-pencil/no-app-imports-in-shared-ui', (sourceRel, _specifier, resolved) => { @@ -233,12 +312,17 @@ const noUiImportsInCore = createImportRule( export const openPencilArchitecturePlugin = { meta: { name: 'open-pencil-architecture', version: '0.0.0' }, ruleDefinitions: [ + strictTestFilePlacement, + noEngineOnlyAssertionsInE2E, + noE2EImportsInEngineTests, noPropertyPanelImportsInCanvas, noAppImportsInWorkspacePackages, noPackageInternalsInApp, noForeignPackageLocalAliases, noAppImportsComponentsOrViews, noComponentsImportViews, + noViewsImportedOutsideEntry, + noNonUiImportsInSharedUi, noAppImportsInSharedUi, noPropertyPanelInternalsOutsidePanel, noUiImportsInCore diff --git a/steiger.config.ts b/steiger.config.ts index f749782e7..c46be47cc 100644 --- a/steiger.config.ts +++ b/steiger.config.ts @@ -12,19 +12,23 @@ export default defineConfig([ 'dist/**', 'desktop/**', 'public/**', - 'tests/**', 'scratch/**', 'demo-recordings/**' ] }, { rules: { + 'open-pencil/strict-test-file-placement': 'error', + 'open-pencil/no-engine-only-assertions-in-e2e': 'error', + 'open-pencil/no-e2e-imports-in-engine-tests': 'error', 'open-pencil/no-property-panel-imports-in-canvas': 'error', 'open-pencil/no-app-imports-in-workspace-packages': 'error', 'open-pencil/no-package-internals-in-app': 'error', 'open-pencil/no-foreign-package-local-aliases': 'error', 'open-pencil/no-app-imports-components-or-views': 'error', 'open-pencil/no-components-import-views': 'error', + 'open-pencil/no-views-imported-outside-entry': 'error', + 'open-pencil/no-non-ui-imports-in-shared-ui': 'error', 'open-pencil/no-app-imports-in-shared-ui': 'error', 'open-pencil/no-property-panel-internals-outside-panel': 'error', 'open-pencil/no-ui-imports-in-core': 'error' diff --git a/tests/engine/scripts/steiger-rules.test.ts b/tests/engine/scripts/steiger-rules.test.ts index 4f0283619..a8cb1510c 100644 --- a/tests/engine/scripts/steiger-rules.test.ts +++ b/tests/engine/scripts/steiger-rules.test.ts @@ -44,6 +44,35 @@ function ruleNamesFor(files: Record) { } describe('OpenPencil architecture rules', () => { + test('enforces test file placement and naming', () => { + expect(ruleNamesFor({ 'tests/e2e/bad.test.ts': '' })).toContain( + 'open-pencil/strict-test-file-placement' + ) + expect(ruleNamesFor({ 'tests/engine/bad.spec.ts': '' })).toContain( + 'open-pencil/strict-test-file-placement' + ) + expect(ruleNamesFor({ 'tests/helpers/setup.ts': '' })).not.toContain( + 'open-pencil/strict-test-file-placement' + ) + }) + + test('blocks engine-only assertions in E2E tests', () => { + const names = ruleNamesFor({ + 'tests/e2e/editor/example.spec.ts': + "import { expect } from 'bun:test'\nimport { helper } from '../../engine/helper'\n" + }) + + expect(names).toContain('open-pencil/no-engine-only-assertions-in-e2e') + }) + + test('blocks E2E imports from engine tests', () => { + const names = ruleNamesFor({ + 'tests/engine/editor/example.test.ts': "import { fixture } from '../../e2e/editor/example.spec'\n" + }) + + expect(names).toContain('open-pencil/no-e2e-imports-in-engine-tests') + }) + test('blocks app code from importing app component layers', () => { const names = ruleNamesFor({ 'src/app/editor/service.ts': "import EditorCanvas from '@/components/EditorCanvas.vue'\n"