From c33882cc002f4e8ec3e6813fa43ad8116b368c51 Mon Sep 17 00:00:00 2001 From: fi3ework Date: Wed, 19 Aug 2026 15:23:32 +0800 Subject: [PATCH] fix(vscode): resolve @rstest/core from the rstack package for bridged projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A folder driven through rstack's Rstest config shim resolved `@rstest/core` with a node_modules walk-up from the config directory. Under pnpm's isolated layout a project that depends on `rstack` alone has no `node_modules/@rstest/core` — the core sits beside `rstack` in the virtual store — so every bridged project failed with "Cannot find @rstest/core" and Rstest was unusable. The lint stack already anchors its core walk-up at the resolved `rstack` directory; the test stack now does the same: - `resolveRstackShim` returns the resolved `rstack` package directory and the bridge threads it into `RstestApi` as the default `@rstest/core` and CLI-bin resolution anchor. Native projects keep the cwd anchor and `rstack.rstest.rstestPackagePath` still overrides. - A missing core is reported once: `createChildProcess` throws a marker error the project initializer no longer re-logs. - E2E fixtures install with pnpm's default isolated layout — the `publicHoistPattern` flags for `@rslint/core` / `@rstest/core` are gone, since they hid this bug (and, without them, the walk-up used to escape the fixture into this repo's own dev copy). The rstest bridge suite now asserts the resolved core lives inside the fixture; the lint bridge suite stages a project-visible `@rslint/core` only for its native-ownership transition test. - Unit tests pin the anchor per project kind (native / bridged / configured) against a `.pnpm`-shaped tree. --- packages/vscode/AGENTS.md | 2 +- .../e2e/lint/suite-bridge/bridge.test.ts | 28 +++++++ .../vscode/e2e/rstest/suite/bridge.test.ts | 14 ++++ packages/vscode/e2e/rstest/suite/helpers.ts | 1 + packages/vscode/e2e/setupFixtures.mjs | 24 ++---- packages/vscode/src/stacks/test/bridge.ts | 16 ++-- .../vscode/src/stacks/test/coreResolution.ts | 12 +++ packages/vscode/src/stacks/test/index.ts | 16 +++- packages/vscode/src/stacks/test/master.ts | 32 +++++-- packages/vscode/src/stacks/test/project.ts | 43 ++++++---- .../vscode/tests/stacks/test/bridge.test.ts | 3 + .../vscode/tests/stacks/test/master.test.ts | 83 ++++++++++++++++++- .../vscode/tests/stacks/test/project.test.ts | 75 +++++++++++++---- 13 files changed, 277 insertions(+), 72 deletions(-) diff --git a/packages/vscode/AGENTS.md b/packages/vscode/AGENTS.md index 78ffd95..a41aa20 100644 --- a/packages/vscode/AGENTS.md +++ b/packages/vscode/AGENTS.md @@ -39,7 +39,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten - **Yarn Plug'n'Play is unsupported by decision, extension-wide.** Every stack resolves through physical `node_modules` (`shared/packageResolve.ts`, `resolution.ts`'s rstack → `@rslint/core` chain, the fmt bin probe, the rstest package lookup) and the lint worker's own `createRequire` from the core directory does too. Lint once carried a `.pnp.cjs` branch for the find-`@rslint/core` hop only; nothing after that hop (config evaluation, plugin resolution, the other stacks) had PnP hooks, so it never produced a working folder, and upstream removed its own PnP path in the same refactor that introduced `corePath`. Real support would be a PnP editor-SDK-shaped project across all three stacks, not a resolver branch — do not reintroduce one. - **A Lint runtime lives as long as a document needs it, and a folder with none is `running: idle`.** Since the #1617 sync, `RuntimeManager` refcounts each runtime by open document: the first document to resolve a core starts one, the last to release it closes it, so a detected folder with nothing open holds zero workers and zero Go processes. That folder still reports `running` — with the detail `idle` — because it is live and will start a runtime on the next `didOpen`; do **not** add a `StackState` kind for it (the shell's status bar and `when` clauses read the kinds, and idle is not a kind of health). A folder's state is the **worst of** its runtimes plus any document whose core resolution currently fails (last-good: that document keeps the runtime it already had), so one failing core is never masked by a healthy sibling — the same invariant fmt pins across folders, applied inside one and across them alike (lint's rank table matches fmt's: `disabled` there means "no `rstack`", not the kill switch). Triggers: the shell's detection pass (which already covers lockfiles) plus one lint-owned watcher on `node_modules/@rslint/core/package.json` — upstream's glob minus the lockfiles detection owns. Failures report through the status only: upstream's `window.showWarningMessage` is dropped, since stacks own no UI chrome. Consequently `whenStackActive('rslint')` means "the controller registered its folders", not "a server is up" — E2E suites open a document and await diagnostics. - The lint worker is deliberately vscode-free so it can move upstream whole. It takes explicit `--core` / `--config` native paths, writes logs only to stderr because stdout is LSP, and owns the Go child plus config/plugin lifecycles. Config edits use `rslint/configRefresh` with the same pinned path; a native ↔ bridged ownership change replaces the whole folder runtime because protocol 2 locks that choice for the process lifetime. -- The test × `rstack.config.*` bridge stays thin on purpose: it points the upstream machinery at rstack's shipped shim and lets the shim interpret the config inside the worker, same as the CLI. Never re-implement rstack config semantics in the extension. +- The test × `rstack.config.*` bridge stays thin on purpose: it points the upstream machinery at rstack's shipped shim and lets the shim interpret the config inside the worker, same as the CLI. Bridged projects resolve `@rstest/core` from the resolved rstack package directory, mirroring lint, so rstack's dependency remains visible under isolated installs. Never re-implement rstack config semantics in the extension. - The fmt stack is an LSP client: one `rs fmt --lsp` server per detected workspace folder, spawned at the **folder root** even when a deeper `rstack.config.*` exists. Deepest-config-wins was removed deliberately — `rs fmt` loads one config from its cwd with no upward walk, so anchoring deeper made the editor disagree with `rs fmt` in a terminal; a subproject that needs its own fmt config becomes its own workspace folder. The stack registers **no** `DocumentFormattingEditProvider`: the client registers the provider from the server's `documentFormattingProvider` capability, and adding one by hand would double-register. A config create/change/delete **restarts** the owning folder's server (the server caches its config for its process lifetime and has no config-change message), which is also why the stack watches `RSTACK_CONFIG_GLOB` itself instead of relying on detection — a detection signature records which config files exist, not their contents. A detection pass keeps healthy servers and restarts failed ones in place (`isFailedFmtState`) — lockfile events notify even when the folder set is unchanged, precisely so a completed install or upgrade is retried without a manual restart. There is no stdin fallback below `SUPPORT_MATRIX.rstack`; that is a version gate, not an omission. **Nested workspace folders are a documented limitation, by decision**: when a folder and its subdirectory are both workspace folders and both detect fmt, the parent's per-folder selector also matches the nested folder's files, and which server VS Code hands the request to is not defined — the supported shape is subprojects as _sibling_ workspace folders (or only the subproject opened), not parent-plus-child. Routing (lint's `WorkspaceDocumentRouter` shape) was considered and deferred. Why all of it: `docs/adr/0002-fmt-lsp-on-user-node-runtime.md`. - fmt importing `stacks/lint/LanguageServerProcessOwner.ts` is not a refactor across the copies: that file has no lint imports and no lint behaviour, it only owns the native children of one language client — including the ones vscode-languageclient's automatic restart creates, which is exactly the leak an ad-hoc copy would reintroduce. Lint's `ManagedLanguageClient` is _restated_ in `stacks/fmt/index.ts` instead, because importing it from `Rslint.ts` would couple fmt to the lint stack's runtime graph. Keep that line where it is: shared process ownership yes, shared stack runtime no. - The VSIX is platform-targeted for exactly one reason: the test stack's AST collection loads a native parser binding. Do not add another native dependency — it multiplies the release matrix. diff --git a/packages/vscode/e2e/lint/suite-bridge/bridge.test.ts b/packages/vscode/e2e/lint/suite-bridge/bridge.test.ts index b23cdc6..64497dd 100644 --- a/packages/vscode/e2e/lint/suite-bridge/bridge.test.ts +++ b/packages/vscode/e2e/lint/suite-bridge/bridge.test.ts @@ -3,6 +3,7 @@ import { spawnSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; import * as vscode from 'vscode'; +import { findPackageJsonUncached } from '../../../src/shared/packageResolve'; import { getRslintDiagnostics, waitForRslintDiagnostics, @@ -60,12 +61,38 @@ suite('Rstack lint bridge', function () { const root = workspaceRoot(); const rstackConfigPath = path.join(root, 'rstack.config.ts'); const nativeConfigPath = path.join(root, nativeConfigName); + const nativeNodeModulesPath = path.join(root, 'node_modules'); const markerPath = path.join(root, '.lint-worker-config.json'); const originalConfig = fs.readFileSync(rstackConfigPath, 'utf8'); + function installNativeCore(): void { + // The fixture intentionally depends only on rstack, so the first two tests + // prove bridged resolution against pnpm's isolated transitive dependency. + // A native config, however, needs its own project-visible @rslint/core. + // Stage that install only for the ownership-transition test, inside the + // sandbox workspace copy (torn down below): link the core rstack itself + // resolves — the same walk the extension performs — so no store layout is + // assumed here. + const rstackPackageJson = findPackageJsonUncached('rstack', root); + assert.ok(rstackPackageJson, 'the fixture install should provide rstack'); + const corePackageJson = findPackageJsonUncached( + '@rslint/core', + path.dirname(rstackPackageJson), + ); + assert.ok(corePackageJson, 'rstack should resolve its @rslint/core'); + const scopeDir = path.join(nativeNodeModulesPath, '@rslint'); + fs.mkdirSync(scopeDir, { recursive: true }); + fs.symlinkSync( + path.dirname(corePackageJson), + path.join(scopeDir, 'core'), + process.platform === 'win32' ? 'junction' : 'dir', + ); + } + teardown(async () => { fs.writeFileSync(rstackConfigPath, originalConfig, 'utf8'); fs.rmSync(nativeConfigPath, { force: true }); + fs.rmSync(nativeNodeModulesPath, { recursive: true, force: true }); fs.rmSync(markerPath, { force: true }); const document = vscode.workspace.textDocuments.find( (candidate) => @@ -142,6 +169,7 @@ suite('Rstack lint bridge', function () { const document = await openLintTarget(); await waitForRslintDiagnostics(document, hasNoDebugger); + installNativeCore(); fs.writeFileSync( nativeConfigPath, `export default [{ rules: { 'no-debugger': 'off' } }];\n`, diff --git a/packages/vscode/e2e/rstest/suite/bridge.test.ts b/packages/vscode/e2e/rstest/suite/bridge.test.ts index f2189fc..b81605d 100644 --- a/packages/vscode/e2e/rstest/suite/bridge.test.ts +++ b/packages/vscode/e2e/rstest/suite/bridge.test.ts @@ -15,6 +15,7 @@ // detection change can deregister and re-register the stack, which publishes a // fresh `TestController` (same reason as `workspace.test.ts`). import assert from 'node:assert'; +import fs from 'node:fs'; import path from 'node:path'; import vscode from 'vscode'; import { @@ -116,6 +117,19 @@ suite('Rstack bridge suite', () => { { label: 'trims a string' }, ]); }); + + const sourceUri = vscode.Uri.file( + path.join(RSTACK_FIXTURE, 'rstack.config.ts'), + ).toString(); + const rstestPath = currentRstestExports().getResolvedRstestPath(sourceUri); + assert.ok(rstestPath, 'the bridged project should resolve @rstest/core'); + // The resolved path is realpath'd; compare against the physical fixture. + assert.ok( + rstestPath.startsWith( + path.join(fs.realpathSync(RSTACK_FIXTURE), 'node_modules'), + ), + `expected the fixture's own @rstest/core, got: ${rstestPath}`, + ); }); test('runs bridged tests through the rstack config shim', async () => { diff --git a/packages/vscode/e2e/rstest/suite/helpers.ts b/packages/vscode/e2e/rstest/suite/helpers.ts index 18cddbb..d2467cb 100644 --- a/packages/vscode/e2e/rstest/suite/helpers.ts +++ b/packages/vscode/e2e/rstest/suite/helpers.ts @@ -14,6 +14,7 @@ import type { RstackExtensionExports } from '../../../src/types'; export interface RstestExports { testController: vscode.TestController; runProfile: vscode.TestRunProfile; + getResolvedRstestPath: (sourceUri: string) => string | undefined; startTestRun: ( request: vscode.TestRunRequest, token: vscode.CancellationToken, diff --git a/packages/vscode/e2e/setupFixtures.mjs b/packages/vscode/e2e/setupFixtures.mjs index caef3a4..c591905 100644 --- a/packages/vscode/e2e/setupFixtures.mjs +++ b/packages/vscode/e2e/setupFixtures.mjs @@ -44,6 +44,10 @@ const install = (name) => { throw new Error(`E2E fixture ${name} has no package.json at ${cwd}`); } console.log(`[e2e] installing fixture: ${name}`); + // Keep pnpm's default isolated layout. In the rstack fixture the tool cores + // are transitive dependencies beside rstack in the virtual store, matching + // the layout users get rather than masking resolution bugs with public + // hoisting. const result = spawnSync( pnpmCommand, [ @@ -55,9 +59,9 @@ const install = (name) => { // the moment a patch release lands. '--no-frozen-lockfile', '--prefer-offline', - // Changing a fixture's install config (its hoist patterns, say) makes - // pnpm want to purge `node_modules`, which it refuses to do without a - // TTY. The directory is disposable. + // Changing a fixture's install config makes pnpm want to purge + // `node_modules`, which it refuses to do without a TTY. The directory is + // disposable. '--config.confirmModulesPurge=false', // Fixtures deliberately install pinned published versions of the Rstack // toolchain, which are often hours old — disable pnpm's @@ -71,20 +75,6 @@ const install = (name) => { // published packages exactly like a user project would, so run their // build scripts as-is. '--config.dangerouslyAllowAllBuilds=true', - // `@rslint/core` / `@rstest/core` may reach a fixture only as transitive - // dependencies of `rstack` (the rstack fixture depends on `rstack` - // alone), yet the extension resolves them with a node_modules walk-up - // from the project dir — which pnpm's isolated store defeats: the - // walk-up would climb out of the fixture and silently find THIS REPO's - // dev copies instead of the published ones. Public-hoisting the two - // reproduces the npm/Yarn layout the extension is designed against, and - // is inert for fixtures that already depend on them directly. It must - // be a CLI flag: pnpm 11 no longer reads `public-hoist-pattern` from a - // fixture-local `.npmrc` (verified — it lands as an empty - // `publicHoistPattern` in `.modules.yaml`), and `--ignore-workspace` - // also ignores a local pnpm-workspace.yaml. - '--config.publicHoistPattern=@rslint/core', - '--config.publicHoistPattern=@rstest/core', ], { cwd, diff --git a/packages/vscode/src/stacks/test/bridge.ts b/packages/vscode/src/stacks/test/bridge.ts index 083695f..581a941 100644 --- a/packages/vscode/src/stacks/test/bridge.ts +++ b/packages/vscode/src/stacks/test/bridge.ts @@ -31,8 +31,9 @@ import { status } from './status'; * the worker's spawn cwd is the only anchor it has. That is why the synthesized * `Project` must carry an explicit cwd (adaptation #5): pointing a `Project` at * the shim without it would cwd the worker into `node_modules/rstack/dist/`, - * where the probe finds nothing and `@rstest/core` would resolve from the wrong - * root. + * where the probe finds nothing. Package resolution is anchored separately at + * the resolved `rstack` directory, where package managers such as pnpm install + * rstack's `@rstest/core` dependency. */ /** Relative to the `rstack` package root. Same file `rs test` injects. */ @@ -41,6 +42,8 @@ const SHIM_RELATIVE_PATH = path.join('dist', 'rstestConfig.js'); export type RstackShim = { /** Absolute path of `/dist/rstestConfig.js`. */ readonly configFilePath: string; + /** Absolute path of the resolved `rstack` package root. */ + readonly packageDirectory: string; /** The installed `rstack` version, when it could be read. */ readonly version?: string; }; @@ -76,10 +79,8 @@ export function resolveRstackShim( return undefined; } - const configFilePath = path.join( - path.dirname(packageJsonPath), - SHIM_RELATIVE_PATH, - ); + const packageDirectory = path.dirname(packageJsonPath); + const configFilePath = path.join(packageDirectory, SHIM_RELATIVE_PATH); if (!existsSync(configFilePath)) { if (!silent) { logger.error( @@ -106,8 +107,9 @@ export function resolveRstackShim( logger.debug('Resolved the rstack Rstest config shim', { configFilePath, + packageDirectory, version, }); status.versionOk(configDir); - return { configFilePath, version }; + return { configFilePath, packageDirectory, version }; } diff --git a/packages/vscode/src/stacks/test/coreResolution.ts b/packages/vscode/src/stacks/test/coreResolution.ts index f33dea4..1ff00bd 100644 --- a/packages/vscode/src/stacks/test/coreResolution.ts +++ b/packages/vscode/src/stacks/test/coreResolution.ts @@ -10,6 +10,18 @@ * does not resolve is a setting the user has to fix, so it is notified. */ +/** + * Resolution failed after the actionable error was already logged or shown. + * Callers still reject so project initialization stops, but must not report the + * same failure again. + */ +export class ReportedRstestResolutionError extends Error { + constructor() { + super('Failed to resolve rstest path'); + this.name = 'ReportedRstestResolutionError'; + } +} + // Whether `specifier` itself is what could not be found. `MODULE_NOT_FOUND` // alone is too broad: a package that is installed but whose entry file is gone // (an interrupted install, or a workspace link that has not been built) throws diff --git a/packages/vscode/src/stacks/test/index.ts b/packages/vscode/src/stacks/test/index.ts index a55bdb3..42d0ff6 100644 --- a/packages/vscode/src/stacks/test/index.ts +++ b/packages/vscode/src/stacks/test/index.ts @@ -73,16 +73,24 @@ class Rstest implements vscode.Disposable { /** * What upstream's `activate()` effectively exported (the `Rstest` instance): * the E2E suites (`e2e/rstest/`) consume `testController`, `runProfile` - * and `startTestRun`. The shell republishes this object through the - * extension's public exports (`RstackExtensionExports.whenStackActive`). - * All three values are stable for the lifetime of one registration; a - * re-registration publishes a fresh object. + * and `startTestRun`, plus the repo-only resolved-path probe used by bridge + * coverage. The shell republishes this object through the extension's public + * exports (`RstackExtensionExports.whenStackActive`). These values are stable + * for the lifetime of one registration; a re-registration publishes a fresh + * object. */ buildExports(): Record { return { testController: this.ctrl, runProfile: this.runProfile, startTestRun: this.startTestRun, + getResolvedRstestPath: (sourceUri: string) => { + for (const workspace of this.workspaces.values()) { + const project = workspace.projects.get(sourceUri); + if (project) return project.api.resolvedRstestPath; + } + return undefined; + }, }; } diff --git a/packages/vscode/src/stacks/test/master.ts b/packages/vscode/src/stacks/test/master.ts index efadfc9..78c7459 100644 --- a/packages/vscode/src/stacks/test/master.ts +++ b/packages/vscode/src/stacks/test/master.ts @@ -21,6 +21,7 @@ import { formatConfiguredCoreNotFoundMessage, formatCoreNotFoundMessage, isModuleNotFoundError, + ReportedRstestResolutionError, } from './coreResolution'; import type { RstestDiagnostics } from './diagnostics'; import type { TestErrorStore } from './errorStore'; @@ -117,12 +118,13 @@ export class RstestApi { // restart — see `reportNodeRuntimeIssue` and the spawn abort in // `createChildProcess`. private disposed = false; + private lastResolvedRstestPath?: string; constructor( private workspace: vscode.WorkspaceFolder, /** - * The worker spawn cwd, the `@rstest/core` resolution root, the terminal - * cwd and the base the terminal's `-c` path is relativized against. + * The worker spawn cwd, the terminal cwd and the base the terminal's `-c` + * path is relativized against. * * The worker-cwd decoupling adaptation: upstream derives this from * `dirname(configFilePath)` inside `Project`. It is now passed in, so the @@ -134,8 +136,19 @@ export class RstestApi { private cwd: string, private configFilePath: string, private project: Project, + /** + * Where the default `@rstest/core` (and CLI bin) walk-up starts. Chosen + * by `Project` — see `ProjectSource.rstestResolutionDir`; an explicit + * `rstestPackagePath` bypasses it. + */ + private rstestResolutionDir: string, ) {} + /** E2E-only probe (`buildExports`): the last successfully resolved `@rstest/core` entry. */ + get resolvedRstestPath(): string | undefined { + return this.lastResolvedRstestPath; + } + /** * The failure-latch key for this master's status reports. The project's * source URI is unique (the projects map is keyed by it), unlike `cwd`, @@ -300,7 +313,7 @@ export class RstestApi { formatConfiguredCoreNotFoundMessage(configuredPackagePath), ); } - logger.error(formatCoreNotFoundMessage(this.cwd)); + logger.error(formatCoreNotFoundMessage(fromDir)); return undefined; } } @@ -340,12 +353,12 @@ export class RstestApi { // answer, while the bare specifier keeps the exports map honored. const found = findPackageJsonUncached( dirname(CORE_PACKAGE_JSON), - this.cwd, + this.rstestResolutionDir, ); if (!found) { // The normal state of a repository whose dependencies are not // installed yet: output channel only, never a notification. - logger.error(formatCoreNotFoundMessage(this.cwd)); + logger.error(formatCoreNotFoundMessage(this.rstestResolutionDir)); return ''; } corePackageJsonPath = found; @@ -387,6 +400,7 @@ export class RstestApi { } } + this.lastResolvedRstestPath = nodeExport; return nodeExport; } catch (e) { vscode.window.showErrorMessage(toErrorMessage(e)); @@ -406,9 +420,11 @@ export class RstestApi { // Same uncached lookup as the worker resolution above. pkgJsonPath = findPackageJsonUncached( dirname(CORE_PACKAGE_JSON), - this.cwd, + this.rstestResolutionDir, ); - if (!pkgJsonPath) logger.error(formatCoreNotFoundMessage(this.cwd)); + if (!pkgJsonPath) { + logger.error(formatCoreNotFoundMessage(this.rstestResolutionDir)); + } } if (!pkgJsonPath) return undefined; const pkg = (readPackageJson(pkgJsonPath) ?? {}) as { @@ -613,7 +629,7 @@ export class RstestApi { } const rstestPath = this.resolveRstestPath(); if (!rstestPath) { - throw new Error('Failed to resolve rstest path'); + throw new ReportedRstestResolutionError(); } const debuggerPort = getConfigValue('debuggerPort', this.workspace); const debuggerAddress = getConfigValue('debuggerAddress', this.workspace); diff --git a/packages/vscode/src/stacks/test/project.ts b/packages/vscode/src/stacks/test/project.ts index 817735e..61849b0 100644 --- a/packages/vscode/src/stacks/test/project.ts +++ b/packages/vscode/src/stacks/test/project.ts @@ -6,6 +6,7 @@ import vscode from 'vscode'; import { RSTACK_CONFIG_NAMES } from '../../detection'; import { resolveRstackShim } from './bridge'; import { watchConfigValue } from './config'; +import { ReportedRstestResolutionError } from './coreResolution'; import { logger } from './logger'; import { RstestApi } from './master'; import { type ChildProjectRef, computeCoveredConfigs } from './projectCoverage'; @@ -42,14 +43,18 @@ export type ProjectSource = { */ readonly configFileUri?: vscode.Uri; /** - * The worker spawn cwd and package-resolution root. Defaults to - * `dirname(configFileUri)` — byte-identical to upstream for a native config. - * The bridge sets it to the `rstack.config.*` directory so the shim's - * single-directory, no-parent-walk `loadRstackConfig()` probe finds the - * config and `@rstest/core` resolves from the project rather than from - * `node_modules/rstack/dist/`. + * The worker spawn cwd. Defaults to `dirname(configFileUri)` — byte-identical + * to upstream for a native config. The bridge sets it to the + * `rstack.config.*` directory so the shim's single-directory, no-parent-walk + * `loadRstackConfig()` probe finds the config. */ readonly cwd?: string; + /** + * The default `@rstest/core` and CLI package-resolution anchor. Defaults to + * `cwd`; the bridge sets it to the already-resolved `rstack` package root so + * rstack's dependency is visible in isolated `node_modules` layouts. + */ + readonly rstestResolutionDir?: string; /** True for a `Project` synthesized by the rstack bridge. */ readonly isBridge?: boolean; }; @@ -232,15 +237,7 @@ export class WorkspaceManager implements vscode.Disposable { for (const [key, project] of [...this.projects]) { if (!project.configLoadFailed) continue; project.dispose(); - this.projects.set( - key, - this.createProject({ - sourceUri: project.sourceUri, - configFileUri: project.configFileUri, - cwd: project.cwd, - isBridge: project.isBridge, - }), - ); + this.projects.set(key, this.createProject(project.source)); } } @@ -276,7 +273,8 @@ export class WorkspaceManager implements vscode.Disposable { * The rstack bridge: when a folder is governed by * `rstack.config.*` and has **no** tool-native config, synthesize a `Project` * per `rstack.config.*` whose config file is rstack's shipped Rstest shim and - * whose cwd is the config's own directory. + * whose cwd is the config's own directory. Its package-resolution anchor is + * the resolved `rstack` directory, where rstack's Rstest dependency lives. * * A native `rstest.config.*` wins **at its own root**: rstack's own * `rs test` would load the shim, which reads `define.test()`, but a @@ -337,6 +335,8 @@ export class WorkspaceManager implements vscode.Disposable { this.reportedShimFailures.delete(cwd); const configFileUri = vscode.Uri.file(shim.configFilePath); const existing = this.projects.get(key); + // `configFileUri` is derived from the shim's package directory, so a + // matching shim path also means an unchanged resolution anchor. if ( existing?.isBridge && existing.configFileUri.toString() === configFileUri.toString() @@ -354,6 +354,7 @@ export class WorkspaceManager implements vscode.Disposable { sourceUri: rstackConfig, configFileUri, cwd, + rstestResolutionDir: shim.packageDirectory, isBridge: true, }); logger.info( @@ -550,10 +551,13 @@ export class Project implements vscode.Disposable { // dependencies not installed yet). `retryFailedProjects` recreates such // projects on the next detection pass. configLoadFailed = false; + /** What this project was built from; `retryFailedProjects` rebuilds from it. */ + readonly source: ProjectSource; // See `ProjectSource`. readonly sourceUri: vscode.Uri; readonly configFileUri: vscode.Uri; readonly cwd: string; + readonly rstestResolutionDir: string; readonly isBridge: boolean; #watch?: vscode.Disposable; constructor( @@ -564,10 +568,12 @@ export class Project implements vscode.Disposable { private onDidChangeTestFiles?: () => void, private onConfigResolved?: () => void, ) { + this.source = source; this.sourceUri = source.sourceUri; this.configFileUri = source.configFileUri ?? source.sourceUri; // use dirname of config file as default root this.cwd = source.cwd ?? path.dirname(this.configFileUri.fsPath); + this.rstestResolutionDir = source.rstestResolutionDir ?? this.cwd; this.isBridge = source.isBridge ?? false; this.root = vscode.Uri.file(this.cwd); this.api = new RstestApi( @@ -575,6 +581,7 @@ export class Project implements vscode.Disposable { this.cwd, this.configFileUri.fsPath, this, + this.rstestResolutionDir, ); this.cancellationSource = new vscode.CancellationTokenSource(); @@ -592,7 +599,9 @@ export class Project implements vscode.Disposable { .catch((error) => { if (this.cancellationSource.token.isCancellationRequested) return; this.configLoadFailed = true; - logger.error('Failed to initialize project config', error); + if (!(error instanceof ReportedRstestResolutionError)) { + logger.error('Failed to initialize project config', error); + } // Let the manager settle its tree even when a config fails to load. this.onConfigResolved?.(); }); diff --git a/packages/vscode/tests/stacks/test/bridge.test.ts b/packages/vscode/tests/stacks/test/bridge.test.ts index b03b582..9ab73e2 100644 --- a/packages/vscode/tests/stacks/test/bridge.test.ts +++ b/packages/vscode/tests/stacks/test/bridge.test.ts @@ -108,6 +108,9 @@ describe('resolveRstackShim', () => { expect(shim?.configFilePath).toBe( path.join(configDir, 'node_modules', 'rstack', 'dist', 'rstestConfig.js'), ); + expect(shim?.packageDirectory).toBe( + path.join(configDir, 'node_modules', 'rstack'), + ); expect(fs.existsSync(shim!.configFilePath)).toBe(true); }); diff --git a/packages/vscode/tests/stacks/test/master.test.ts b/packages/vscode/tests/stacks/test/master.test.ts index 145d9ea..296b2d0 100644 --- a/packages/vscode/tests/stacks/test/master.test.ts +++ b/packages/vscode/tests/stacks/test/master.test.ts @@ -121,7 +121,7 @@ afterEach(() => { for (const key of Object.keys(settings)) delete settings[key]; }); -const createApi = (cwd = noCoreDir) => { +const createApi = (cwd = noCoreDir, rstestResolutionDir = cwd) => { const workspace = { uri: { fsPath: cwd } }; // `sourceUri` backs the per-project status latch key, which the version // check on the spawn path reads before anything can fail. @@ -131,9 +131,90 @@ const createApi = (cwd = noCoreDir) => { cwd, `${cwd}/rstest.config.ts`, project as any, + rstestResolutionDir, ); }; +const writeCoreInstall = (root: string) => { + const packageDir = path.join(root, 'node_modules', '@rstest', 'core'); + const entry = path.join(packageDir, 'index.js'); + const bin = path.join(packageDir, 'bin', 'rstest.js'); + fs.mkdirSync(path.dirname(bin), { recursive: true }); + fs.writeFileSync( + path.join(packageDir, 'package.json'), + JSON.stringify({ + name: '@rstest/core', + version: '0.11.8', + main: 'index.js', + bin: { rstest: 'bin/rstest.js' }, + }), + ); + fs.writeFileSync(entry, 'module.exports = {};\n'); + fs.writeFileSync(bin, '#!/usr/bin/env node\n'); + return { packageDir, entry, bin }; +}; + +const resolveRstestPaths = (api: RstestApi) => ({ + entry: (api as any).resolveRstestPath() as string, + bin: (api as any).resolveRstestBin() as string, +}); + +describe('RstestApi package-resolution anchor', () => { + let root: string; + let cwd: string; + /** pnpm's virtual-store entry: `rstack` and its `@rstest/core` are siblings here. */ + let storeEntry: string; + let rstackDir: string; + + beforeEach(() => { + root = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), 'rstest-resolution-')), + ); + cwd = path.join(root, 'app'); + storeEntry = path.join(cwd, 'node_modules', '.pnpm', 'rstack@0.6.1'); + rstackDir = path.join(storeEntry, 'node_modules', 'rstack'); + fs.mkdirSync(rstackDir, { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(root, { recursive: true, force: true }); + }); + + it('keeps native projects anchored at their cwd', () => { + const native = writeCoreInstall(cwd); + writeCoreInstall(storeEntry); + + expect(resolveRstestPaths(createApi(cwd))).toEqual({ + entry: native.entry, + bin: native.bin, + }); + }); + + it('anchors bridged projects at the resolved rstack directory', () => { + writeCoreInstall(cwd); + const bridged = writeCoreInstall(storeEntry); + + expect(resolveRstestPaths(createApi(cwd, rstackDir))).toEqual({ + entry: bridged.entry, + bin: bridged.bin, + }); + }); + + it('lets rstestPackagePath override the bridge anchor', () => { + writeCoreInstall(storeEntry); + const configured = writeCoreInstall(path.join(root, 'configured')); + settings.rstestPackagePath = path.join( + configured.packageDir, + 'package.json', + ); + + expect(resolveRstestPaths(createApi(cwd, rstackDir))).toEqual({ + entry: configured.entry, + bin: configured.bin, + }); + }); +}); + describe('RstestApi with a missing @rstest/core', () => { beforeEach(() => { shownMessages.length = 0; diff --git a/packages/vscode/tests/stacks/test/project.test.ts b/packages/vscode/tests/stacks/test/project.test.ts index d2897f1..6957972 100644 --- a/packages/vscode/tests/stacks/test/project.test.ts +++ b/packages/vscode/tests/stacks/test/project.test.ts @@ -1,14 +1,21 @@ import path from 'node:path'; -import { describe, expect, it, rs } from '@rstest/core'; +import { afterEach, beforeEach, describe, expect, it, rs } from '@rstest/core'; +import { ReportedRstestResolutionError } from '../../../src/stacks/test/coreResolution'; +import { logger } from '../../../src/stacks/test/logger'; // The worker-cwd decoupling adaptation pinned at its only site: upstream derived the // worker spawn cwd inside `Project` as `dirname(configFileUri)`, so a `Project` // pointing at rstack's shim would have cwd'd into `node_modules/rstack/dist/`. -// The three values `RstestApi` is constructed with are therefore what this test -// asserts — they are the spawn cwd, the `@rstest/core` resolution root and the -// config file Rstest is asked to load. +// The values `RstestApi` is constructed with are therefore what this test +// asserts: the spawn cwd, the independently selected `@rstest/core` resolution +// root and the config file Rstest is asked to load. -const apiCalls: { cwd: string; configFilePath: string }[] = []; +const apiCalls: { + cwd: string; + configFilePath: string; + rstestResolutionDir: string; +}[] = []; +let normalizedConfigFailure: unknown; rs.mock('../../../src/stacks/test/master', () => { class RstestApi { @@ -17,12 +24,16 @@ rs.mock('../../../src/stacks/test/master', () => { cwd: string, configFilePath: string, _project: unknown, + rstestResolutionDir: string, ) { - apiCalls.push({ cwd, configFilePath }); + apiCalls.push({ cwd, configFilePath, rstestResolutionDir }); } // Never settles: the constructor's config-resolution continuation would // otherwise start watchers this test has no filesystem for. getNormalizedConfig() { + if (normalizedConfigFailure) { + return Promise.reject(normalizedConfigFailure); + } return new Promise(() => {}); } dispose() {} @@ -30,6 +41,18 @@ rs.mock('../../../src/stacks/test/master', () => { return { RstestApi, runningWorkers: new Set() }; }); +// One log-channel double for both the vscode mock and `logger.bind`, so the +// assertions below observe every stack log line. +const loggedErrors: string[] = []; +const channel = { + debug: () => {}, + info: () => {}, + warn: () => {}, + error: (message: string) => loggedErrors.push(message), + show: () => {}, + dispose: () => {}, +}; + rs.mock('vscode', () => { const vscode = { Uri: { @@ -54,14 +77,7 @@ rs.mock('vscode', () => { ) {} }, window: { - createOutputChannel: () => ({ - debug: () => {}, - info: () => {}, - warn: () => {}, - error: () => {}, - show: () => {}, - dispose: () => {}, - }), + createOutputChannel: () => channel, }, workspace: { fs: {}, @@ -106,6 +122,16 @@ const collection = { forEach: () => {}, } as any; +beforeEach(() => { + normalizedConfigFailure = undefined; + loggedErrors.length = 0; + logger.bind(channel as never); +}); + +afterEach(() => { + logger.unbind(); +}); + const createProject = async (source: any) => { apiCalls.length = 0; const { Project } = await import('../../../src/stacks/test/project'); @@ -113,7 +139,7 @@ const createProject = async (source: any) => { return { project, api: apiCalls[0]! }; }; -describe('Project config/cwd decoupling', () => { +describe('Project config/cwd/package-resolution decoupling', () => { it('keeps the upstream derivation for a native rstest config', async () => { const configFile = uri(path.join('/repo', 'pkg', 'rstest.config.ts')); @@ -121,6 +147,7 @@ describe('Project config/cwd decoupling', () => { // Byte-identical to upstream: cwd is the config file's directory. expect(api.cwd).toBe(path.join('/repo', 'pkg')); + expect(api.rstestResolutionDir).toBe(path.join('/repo', 'pkg')); expect(api.configFilePath).toBe(configFile.fsPath); expect(project.configFilePath).toBe(configFile.fsPath); expect(project.sourceUri.toString()).toBe(configFile.toString()); @@ -140,20 +167,22 @@ describe('Project config/cwd decoupling', () => { 'rstestConfig.js', ), ); + const rstackDir = path.dirname(path.dirname(shim.fsPath)); const { project, api } = await createProject({ sourceUri: rstackConfig, configFileUri: shim, cwd: path.join('/repo', 'pkg'), + rstestResolutionDir: rstackDir, isBridge: true, }); // The whole point: `dirname(configFile)` would be // `/node_modules/rstack/dist`, where the shim's single-directory, - // no-parent-walk `loadRstackConfig()` probe finds nothing and - // `@rstest/core` would resolve from rstack's own dependency tree. + // no-parent-walk `loadRstackConfig()` probe finds nothing. expect(api.cwd).toBe(path.join('/repo', 'pkg')); expect(api.cwd).not.toBe(path.dirname(shim.fsPath)); + expect(api.rstestResolutionDir).toBe(rstackDir); // Rstest is still handed the shim as an ordinary JS config file. expect(api.configFilePath).toBe(shim.fsPath); expect(project.configFilePath).toBe(shim.fsPath); @@ -163,4 +192,16 @@ describe('Project config/cwd decoupling', () => { expect(project.isBridge).toBe(true); expect(project.root.fsPath).toBe(path.join('/repo', 'pkg')); }); + + it('does not re-log a package-resolution failure that was already reported', async () => { + normalizedConfigFailure = new ReportedRstestResolutionError(); + + const { project } = await createProject({ + sourceUri: uri(path.join('/repo', 'pkg', 'rstest.config.ts')), + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(project.configLoadFailed).toBe(true); + expect(loggedErrors).toEqual([]); + }); });