test: resolve repository files without climbing directories (#946)

* test: resolve repository files without climbing directories

Twelve tests and helpers reached shared fixtures, package assets, and workers with ../.. paths from import.meta, which the import rule does not see. They now go through repoPath and testPath, a workspaceRoot() that finds the root by its lockfile, the core package's own root, or the #core alias through import.meta.resolve. The root finder derives its folder from import.meta.url, so Playwright specs running under Node can use the helpers too. open-pencil/no-deep-parent-relative-paths rejects climbing two levels in new URL(…, import.meta.url) and in path calls that start from import.meta.

* fix(lint): catch Windows separators and wrapped import.meta paths, and stop at template expressions

The path rule missed '..\..' and a base such as dirname(fileURLToPath(import.meta.url)), and read `../${folder}` followed by '..' as climbing two levels.
This commit is contained in:
Danila Poyarkov 2026-10-07 12:35:39 +00:00 committed by GitHub
parent 11e708de39
commit cff091bc6a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 185 additions and 24 deletions

View file

@ -46,7 +46,7 @@ Before a PR run `bun run check`, `bun run format`, `bun run test:unit`, and `bun
- Across package/app boundaries import the owning package's public exports, never workspace internals or forwarding-only shims. `@open-pencil/scene-graph` owns graph types and primitives; `@open-pencil/kiwi` owns low-level Kiwi/FIG helpers; `@open-pencil/core` provides the compatibility barrel plus the subpaths listed in `packages/core/package.json`.
- `bun run check:arch` enforces: public workspace exports, framework-neutral Core, no app services in views or shared UI, property-panel internals scoped to that panel.
- Package aliases are `#core/*`, `#fig/*`, `#vue/*`, `#cli/*`, `#mcp/*`, `#dom-css/*`, `#design-jsx/*`, `#emit/*`; a package's own tests use `#core-tests/*` and `#fig-tests/*`; the app uses `@/`. Prefer clear relative imports nearby. Never escape an alias root with `../` (for example `#tests/../vite`); fix module ownership instead.
- Never drill with `../../`: one `../` to a sibling folder is fine, two or more means an alias. This holds in tests as much as in source — a test reaches its package's source through `#<pkg>/*` and its own helpers through `#<pkg>-tests/*`. `open-pencil/no-deep-parent-relative-imports` enforces it; a new test directory must be added to `lint:structure` so the rule reaches it.
- Never drill with `../../`: one `../` to a sibling folder is fine, two or more means an alias. This holds in tests as much as in source — a test reaches its package's source through `#<pkg>/*` and its own helpers through `#<pkg>-tests/*`. `open-pencil/no-deep-parent-relative-imports` enforces it for imports and `open-pencil/no-deep-parent-relative-paths` for paths built from `import.meta`; a new test directory must be added to `lint:structure` so the rules reach it.
- Reuse named types from `@open-pencil/scene-graph`; do not respell `Color`, `Vector`, `SceneNode`, `Effect`, `Fill`, or `Stroke`.
## Code conventions

View file

@ -180,6 +180,7 @@
"open-pencil/no-core-framework-imports": "error",
"open-pencil/no-direct-storage-access": "error",
"open-pencil/no-deep-parent-relative-imports": "error",
"open-pencil/no-deep-parent-relative-paths": "error",
"open-pencil/no-broad-double-cast": "error",
"open-pencil/no-unknown-record-double-cast": "error",
"open-pencil/no-broad-unknown-type-assertions": "error",

View file

@ -1,7 +1,17 @@
import { resolve } from 'node:path'
import { existsSync } from 'node:fs'
import { dirname, join } from 'node:path'
import { fileURLToPath } from 'node:url'
/** The CLI entry point, for tests that run it as a subprocess. */
export const CLI_ENTRY = Bun.resolveSync('#cli/index.ts', import.meta.dir)
/** The workspace root, found by its lockfile rather than by counting directories. */
export function workspaceRoot(from = dirname(fileURLToPath(import.meta.url))): string {
for (let dir = from; ; dir = dirname(dir)) {
if (existsSync(join(dir, 'bun.lock'))) return dir
if (dirname(dir) === dir) throw new Error(`No workspace root above ${from}`)
}
}
/** Fixtures shared across the repository; a module import would escape the package root. */
export const FIXTURES = resolve(import.meta.dir, '../../../../tests/fixtures')
export const FIXTURES = join(workspaceRoot(), 'tests/fixtures')

View file

@ -10,9 +10,11 @@ import {
import type { NodeChange } from '@open-pencil/kiwi/fig/codec'
import * as v from 'valibot'
import { FIXTURES } from '../paths'
import { collectAllNodes } from './traversal'
export const FIXTURES = resolve(import.meta.dir, '../../../../../tests/fixtures')
export { FIXTURES }
export const VALID_NODE_TYPES = new Set<string>([
'CANVAS',

View file

@ -0,0 +1,19 @@
import { existsSync } from 'node:fs'
import { dirname, join } from 'node:path'
import { fileURLToPath } from 'node:url'
/** The workspace root, found by its lockfile rather than by counting directories. */
export function workspaceRoot(from = dirname(fileURLToPath(import.meta.url))): string {
for (let dir = from; ; dir = dirname(dir)) {
if (existsSync(join(dir, 'bun.lock'))) return dir
if (dirname(dir) === dir) throw new Error(`No workspace root above ${from}`)
}
}
/** Fixtures shared across the repository; a module import would escape the package root. */
export const FIXTURES = join(workspaceRoot(), 'tests/fixtures')
/** A file inside this package, for assets that are not modules. */
export function corePackagePath(...segments: string[]): string {
return join(dirname(Bun.resolveSync('@open-pencil/core/package.json', import.meta.dir)), ...segments)
}

View file

@ -10,8 +10,9 @@ import { expectDefined } from '#core-tests/helpers/assert'
import { HEAVY_TEST_TIMEOUT_MS } from '#core-tests/helpers/test-utils'
import { FIXTURES } from '#core-tests/helpers/fig/fixtures'
import { corePackagePath } from '#core-tests/helpers/paths'
const INTER_ASSETS = resolve(import.meta.dir, '../../../../../assets')
const INTER_ASSETS = corePackagePath('assets')
function countGlyphBlobs(bytes: Uint8Array) {
const parsed = parseFigBuffer(new Uint8Array(bytes).buffer)

View file

@ -15,7 +15,7 @@ test.each(['first-page', 'none'] as const)(
graph.createNode('TEXT', graph.addPage('Second').id, { text: 'Second' })
const bytes = await exportFigFile(graph)
const worker = new Worker(
new URL('../../../../../src/kiwi/fig/session/worker.ts', import.meta.url),
import.meta.resolve('#core/kiwi/fig/session/worker'),
{ type: 'module' }
)
const channel = new MessageChannel()

View file

@ -1,8 +1,9 @@
import { readFileSync } from 'node:fs'
import { resolve } from 'node:path'
/** Shared archives live outside the package; this is the only path that reaches them. */
export const FIXTURES = resolve(import.meta.dir, '../../../../tests/fixtures')
import { FIXTURES } from './paths'
export { FIXTURES }
export function readFixtureBytes(name: string): Uint8Array {
return readFileSync(resolve(FIXTURES, name))

View file

@ -0,0 +1,14 @@
import { existsSync } from 'node:fs'
import { dirname, join } from 'node:path'
import { fileURLToPath } from 'node:url'
/** The workspace root, found by its lockfile rather than by counting directories. */
export function workspaceRoot(from = dirname(fileURLToPath(import.meta.url))): string {
for (let dir = from; ; dir = dirname(dir)) {
if (existsSync(join(dir, 'bun.lock'))) return dir
if (dirname(dir) === dir) throw new Error(`No workspace root above ${from}`)
}
}
/** Shared archives live outside the package; this is the only path that reaches them. */
export const FIXTURES = join(workspaceRoot(), 'tests/fixtures')

View file

@ -7,6 +7,7 @@
- Package-local tests mirror source domains under `packages/<owner>/tests/`; central app tests mirror `src/app/**` under `tests/app/`; `tests/integration/` requires a genuinely cross-owner contract; E2E under `tests/e2e/` follows user workflows; native and Figma acceptance are explicit exceptions.
- Existing `tests/engine/**` domains migrate together with runner discovery: `tools/dev/unit-tests/src/shards.ts` lists each owner's canonical home and its current `tests/engine` directories, so a move is a `git mv` plus imports. `bun run check:test-homes` rejects any test added under `tests/engine` and any stale entry in `tools/checks/test-homes/engine-baseline.txt`; new tests go to the canonical home, and a moved file is removed from the baseline (`--write` regenerates it). Scene Graph has migrated to `packages/scene-graph/tests` and the editor domain to `packages/core/tests/editor`; put new Core tests there, mirroring `packages/core/src`.
- Address a package's source by `#<pkg>/*` and its own helpers by `#<pkg>-tests/*` (`#core-tests/*`, `#fig-tests/*`); never drill with `../../`. Registering a new alias means `imports` in the package manifest and `PACKAGE_ALIASES`/`PACKAGE_ALIAS_OWNERS` in `tools/checks/architecture/src/steiger-rules/support.ts`.
- Reach repository files, such as `tests/fixtures`, through `repoPath`/`testPath` in `tests/helpers/paths.ts`, or a package helper's `workspaceRoot()`, which finds the root by its lockfile; never climb from `import.meta` (`open-pencil/no-deep-parent-relative-paths`).
- Owner-local helpers and fixtures stay local; only genuinely shared support goes under central `tests/helpers/<domain>/` and `tests/fixtures/`. MCP transport tests: `tests/engine/mcp/{server,stdio,transport}` with `tests/helpers/mcp`.
- Never commit temporary, diagnostic, or profile specs; keep them in ignored `scratch/`.

View file

@ -1,10 +1,9 @@
import { fileURLToPath } from 'node:url'
import { expect, test } from '@playwright/test'
import type * as AppTabs from '@/app/tabs'
import { CanvasHelper } from '#tests/helpers/canvas'
import { testPath } from '#tests/helpers/paths'
const FIXTURE = 'gold-preview.fig'
@ -14,7 +13,7 @@ const FIXTURE = 'gold-preview.fig'
test('documents closed in tabs are released', async ({ page }) => {
test.setTimeout(90_000)
await page.route(`**/__fixtures/${FIXTURE}`, (route) =>
route.fulfill({ path: fileURLToPath(new URL(`../../fixtures/${FIXTURE}`, import.meta.url)) })
route.fulfill({ path: testPath('fixtures', FIXTURE) })
)
await page.goto('/?test')
await new CanvasHelper(page).waitForInit()

View file

@ -1,10 +1,9 @@
import { fileURLToPath } from 'node:url'
import type { Page } from '@playwright/test'
import { expect, test } from '#tests/e2e/fixtures'
import { expectDefined } from '#tests/helpers/assert'
import { CanvasHelper } from '#tests/helpers/canvas'
import { testPath } from '#tests/helpers/paths'
// This size exceeds the 3x overscan budget and previously produced fractional
// raster offsets, even though the live scene itself was pixel-aligned.
@ -80,7 +79,7 @@ for (const imported of [false, true]) {
test.setTimeout(45_000)
await page.route('**/__fixtures/gold-preview.fig', (route) =>
route.fulfill({
path: fileURLToPath(new URL('../../fixtures/gold-preview.fig', import.meta.url)),
path: testPath('fixtures/gold-preview.fig'),
contentType: 'application/octet-stream'
})
)

View file

@ -1,6 +1,7 @@
import { describe, test, expect, beforeAll, setDefaultTimeout } from 'bun:test'
import { readFileSync } from 'node:fs'
import { resolve } from 'node:path'
import { pathToFileURL } from 'node:url'
import { unzipSync } from 'fflate'
@ -12,11 +13,12 @@ import {
SceneGraph
} from '@open-pencil/core'
import { coreSourcePath, testPath } from '#tests/helpers/paths'
import { heavy } from '#tests/helpers/test-utils'
setDefaultTimeout(30_000)
const FIXTURES = resolve(import.meta.dir, '../../../../fixtures')
const FIXTURES = testPath('fixtures')
const CUSTOM_FIG_KIWI_VERSION = 77
function canvasFigVersion(figData: Uint8Array): number {
@ -38,10 +40,9 @@ function compressInWorker(message: {
figKiwiVersion?: number
}): Promise<Uint8Array> {
return new Promise((resolve, reject) => {
const worker = new Worker(
new URL('../../../../../packages/core/src/io/formats/fig/export-worker.ts', import.meta.url),
{ type: 'module' }
)
const worker = new Worker(pathToFileURL(coreSourcePath('io/formats/fig/export-worker.ts')), {
type: 'module'
})
worker.onmessage = (event: MessageEvent<Uint8Array>) => {
worker.terminate()
resolve(event.data)

View file

@ -1,11 +1,12 @@
import { expect, test } from 'bun:test'
import { readFileSync } from 'node:fs'
import { resolve } from 'node:path'
import { parseFigBuffer } from '@open-pencil/fig'
import type { FigPageManifestEntry } from '@open-pencil/kiwi/fig'
const fixturePath = resolve(import.meta.dir, '../../../fixtures/gold-preview.fig')
import { testPath } from '#tests/helpers/paths'
const fixturePath = testPath('fixtures/gold-preview.fig')
test('reports FIG pages before materializing NodeChange objects', () => {
const bytes = readFileSync(fixturePath)

View file

@ -10,9 +10,10 @@ import {
type SceneNode
} from '@open-pencil/core'
import { testPath } from '../paths'
import { collectAllNodes } from './traversal'
export const FIXTURES = resolve(import.meta.dir, '../../fixtures')
export const FIXTURES = testPath('fixtures')
export const VALID_NODE_TYPES = new Set<string>([
'CANVAS',

View file

@ -1,7 +1,16 @@
import { existsSync } from 'node:fs'
import { join } from 'node:path'
import { dirname, join } from 'node:path'
import { fileURLToPath } from 'node:url'
const repoRoot = join(import.meta.dir, '..', '..')
/** The workspace root, found by its lockfile rather than by counting directories. */
export function workspaceRoot(from = dirname(fileURLToPath(import.meta.url))): string {
for (let dir = from; ; dir = dirname(dir)) {
if (existsSync(join(dir, 'bun.lock'))) return dir
if (dirname(dir) === dir) throw new Error(`No workspace root above ${from}`)
}
}
const repoRoot = workspaceRoot()
export function repoPath(...segments: string[]): string {
return join(repoRoot, ...segments)

View file

@ -374,6 +374,7 @@ import {
noVueSelfPackageImports,
noCrossPackageSourceImports,
noDeepParentRelativeImports,
noDeepParentRelativePaths,
noCoreParentRelativeImports,
noMcpParentRelativeImports,
noVueParentRelativeImports,
@ -452,6 +453,7 @@ const plugin = {
'no-vue-self-package-imports': noVueSelfPackageImports,
'no-cross-package-source-imports': noCrossPackageSourceImports,
'no-deep-parent-relative-imports': noDeepParentRelativeImports,
'no-deep-parent-relative-paths': noDeepParentRelativePaths,
'no-core-parent-relative-imports': noCoreParentRelativeImports,
'no-mcp-parent-relative-imports': noMcpParentRelativeImports,
'no-vue-parent-relative-imports': noVueParentRelativeImports,

View file

@ -88,6 +88,71 @@ const noDeepParentRelativeImports = createParentRelativeImportRule({
minDepth: 2
})
/**
* A path that climbs two or more directories, written as `../..`, `..\\..`, or `'..', '..'`.
* An unknown part, such as a template expression, ends the run of parent segments.
*/
function climbsTwoLevels(values: (string | null)[]): boolean {
const joined = values
.map((value) => value ?? '\u0000')
.join('/')
.replaceAll('\\', '/')
return /(^|\/)\.\.\/+\.\.(\/|$)/.test(joined)
}
function staticString(node: TSESTree.Node): string | null {
if (node.type === 'Literal' && typeof node.value === 'string') return node.value
if (node.type === 'TemplateLiteral') {
const head = node.quasis[0]?.value.cooked ?? null
return head !== null && node.expressions.length ? `${head}\u0000` : head
}
return null
}
/** `import.meta.url`, `.dir`, or `.dirname`, directly or wrapped, as in `dirname(fileURLToPath(import.meta.url))`. */
function isImportMetaLocation(node: TSESTree.Node): boolean {
if (node.type === 'MemberExpression') {
if (node.object.type === 'MetaProperty') {
return (
node.property.type === 'Identifier' &&
['url', 'dir', 'dirname'].includes(node.property.name)
)
}
return isImportMetaLocation(node.object)
}
if (node.type === 'CallExpression') return node.arguments.some(isImportMetaLocation)
return false
}
const noDeepParentRelativePaths: RuleDefinition = {
meta: {
docs: {
description:
'Disallow climbing two or more directories from import.meta — use a path helper or alias'
}
},
create(context: RuleContext) {
const message =
'Resolve the file through a path helper such as repoPath() or an import alias instead of climbing with ../..'
function check(node: TSESTree.Node, relative: TSESTree.CallExpressionArgument[]) {
if (relative.length && climbsTwoLevels(relative.map(staticString))) {
context.report({ node, message })
}
}
return {
NewExpression(node) {
if (node.callee.type !== 'Identifier' || node.callee.name !== 'URL') return
if (!node.arguments[1] || !isImportMetaLocation(node.arguments[1])) return
check(node, node.arguments.slice(0, 1))
},
CallExpression(node) {
const start = node.arguments.findIndex(isImportMetaLocation)
if (start !== -1) check(node, node.arguments.slice(start + 1))
}
}
}
}
const noCoreParentRelativeImports = createParentRelativeImportRule({
description: 'Disallow parent-relative imports in core internals — use #core/* aliases',
applies: (file) =>
@ -214,6 +279,7 @@ export {
noVueSelfPackageImports,
noCrossPackageSourceImports,
noDeepParentRelativeImports,
noDeepParentRelativePaths,
noCoreParentRelativeImports,
noMcpParentRelativeImports,
noVueParentRelativeImports,

View file

@ -0,0 +1,34 @@
import { describe, expect, test } from 'bun:test'
import { lint, ruleDiagnostics } from './helpers/lint.ts'
const rule = 'no-deep-parent-relative-paths'
const rules = { [`open-pencil/${rule}`]: 'error' }
// A template that climbs one level and then names a folder, assembled so it stays source text.
const oneLevelTemplate = ['join(import.meta.dir, `../', '$', "{folder}`, '..', 'assets')"].join('')
describe('no-deep-parent-relative-paths', () => {
test.each([
"new URL('../../fixtures/a.fig', import.meta.url)",
'new URL(`../../fixtures/a.fig`, import.meta.url)',
"resolve(import.meta.dir, '../../../tests/fixtures')",
"join(import.meta.dirname, '..', '..', 'assets')",
"path.resolve(import.meta.dir, '../..')",
"join(import.meta.dir, '..\\\\..\\\\assets')",
"join(dirname(fileURLToPath(import.meta.url)), '../../fixtures')"
])('rejects %s', async (source) => {
expect(ruleDiagnostics(await lint(source, rules), rule)).toHaveLength(1)
})
test.each([
"new URL('../fixtures/a.fig', import.meta.url)",
"new URL('./worker.ts', import.meta.url)",
"join(import.meta.dir, '..', 'fixtures')",
"credentialRef('../../other-app', 'api-key')",
"resolve(root, '../../somewhere')",
"join(dirname(fileURLToPath(import.meta.url)), '..', 'fixtures')",
oneLevelTemplate
])('accepts %s', async (source) => {
expect(ruleDiagnostics(await lint(source, rules), rule)).toHaveLength(0)
})
})