chore: enforce test structure rules
This commit is contained in:
Danila Poyarkov
parent
c80ce4c7a9
commit
8976ea7c3d
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
|
|
|
|||
|
|
@ -44,6 +44,35 @@ function ruleNamesFor(files: Record<string, string>) {
|
|||
}
|
||||
|
||||
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"
|
||||
|
|
|
|||
Loading…
Reference in a new issue