Skip to content

Commit c02ba52

Browse files
authored
fix(vscode): route merged single-file test runs to the deepest project (#60)
* fix(vscode): route merged single-file test runs to the deepest project * refactor(vscode): route by published item roots and report dropped items skipped * fix(vscode): route test runs by file or case identity, not file URI alone * fix(vscode): include suite/case type in the run routing key
1 parent 388dbbb commit c02ba52

17 files changed

Lines changed: 390 additions & 32 deletions

File tree

‎CONTEXT.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ Glossary of terms used across rstack-editor. Code, docs, commit messages and rev
2727
- **Config root** — the directory a tool's config is loaded from, which is also the directory the tool's process stands in. For the fmt server the editor anchors it at the workspace folder root, so it loads the config a terminal opened on that folder would, and a subproject that needs its own config becomes its own workspace folder. The test stack does not share this anchor: a project's cwd is set per project (for native configs, upstream's config-file-directory rule). _Avoid_: config directory, project root.
2828
- **Ownership** — the editor-side rule choosing one config source for a tool's unit of work when both a native config and a Rstack config are present: the atomic tool's native config wins and the bridge yields. The unit is the tool's own — a project for test (one per config directory), a workspace folder for lint (one config choice per folder, locked for each lint runtime's lifetime). This rule exists only in the editor; upstream CLIs never face the choice, since each reads only its own config.
2929

30+
## test
31+
32+
- **Test file owner** — for one run request, a project whose root is deepest among the published items sharing a file URI, with ties sharing ownership. This per-request choice routes single-file/case selections across projects; explicit single-project selections and project-wide runs retain CLI scope.
33+
3034
## lint
3135

3236
- **Rslint core** — one `@rslint/core` package directory, identified by its real path (two copies of the same version are two cores; a symlink to one copy is that copy). Everything a lint runtime runs — the Go binary, config host, protocol version, plugin host — derives from one Rslint core. _Avoid_: core (bare), installation, binary.

‎packages/vscode/AGENTS.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
55
## The copies are intentional
66

77
- `stacks/lint` and `stacks/test` are deliberate near-verbatim copies of the upstream extensions, kept close to upstream so changes can be synced by diffing. Do NOT deduplicate or refactor across the two stacks — the duplication is the point; consolidation is a later, explicit phase.
8-
- The copies diverge from upstream in exactly nine ways (the "adaptations" below). When syncing upstream, preserve them. A tenth divergence is either a bug or must be added to this list.
8+
- The copies diverge from upstream in exactly ten ways (the "adaptations" below). When syncing upstream, preserve them. An eleventh divergence is either a bug or must be added to this list.
99
- **Tracked upstream state.** `stacks/lint` is synced to web-infra-dev/rslint `packages/vscode-extension` at **39536fd6** (#1617 — per-document core resolution, `CoreResolver` + `RuntimeManager`, `corePath`, PnP removed) and **892482e0** (#1630 — `configPath` on `rslint/configRefresh`). Targeted later ports are **84f9c9b5** (#1967 — languageclient-owned live LSP tracing) and **b7176723** (#1951 — remove legacy JSON config watching); the Unicode BOM E2E comes from **5fc197a5** (#1560), with its native-config fixture shape from **b7176723**. `CoreResolver.ts` / `RuntimeManager.ts` / `WorkspaceDocumentRouter.ts` / `Rslint.ts` are the files to diff when syncing further; record the new commits here when you do.
1010
- **Ahead of upstream — offer these back when syncing** (bug fixes, not adaptations): (1) `RuntimeManager.reconcile` resolves the document's core **before** sweeping pending uses (`planDocumentCore`), so a reconcile landing on the key a pending start is already producing adopts that start instead of tearing it down mid-`initialize` — the teardown made vscode-languageclient force-notify ("couldn't create connection to server") whenever the register-time pass, a detection change and `didOpen` landed inside one worker startup window (`tests/stacks/lint/runtimeManager.test.ts`). (2) `Rslint.close()` gives a still-Starting language client a bounded chance to settle before tearing down its transport, so a legitimate mid-start close (document closed during start, core key changed) stops cleanly instead of triggering the same force-notified toasts. (3) The registry-harness E2E gives its never-settling startup operation 500ms to begin and accepts only the in-flight timeout message, so a stalled runner cannot satisfy the assertion through the already-expired path (`e2e/lint/suite/registry-harness.test.ts`). (4) `Project.retryFailedConfig()` keeps a failed Rstest project and retries its config evaluation in place with one single-flight promise, so repeated dependency-change passes neither overlap workers nor repeat an unchanged not-installed warning. (5) `RuntimeManager` retires a stopped client even when its resolved key is unchanged. The existing closing barrier and pending-use adoption share one replacement across documents; running and starting clients remain untouched (`tests/stacks/lint/runtimeManager.test.ts`).
1111

1212
- **Targeted Rstest lifecycle port:** `RstestApi.getNormalizedConfig()` closes its worker in `finally`, including rejected config evaluation, matching web-infra-dev/rstest `packages/vscode/src/master.ts` at `d82db4fc31a61ee74b2a74917f14a458e1bca419`. This fixes a leak in our older copy; it is already fixed upstream. Dependency passes retry failed projects, including real config errors, while preserving single-flight loading and worker cleanup.
1313

14-
## The nine adaptations
14+
## The ten adaptations
1515

1616
1. **Shell activation** — stacks never self-activate; `register()` returns fast and never blocks on starting a server/worker.
1717
2. **Namespace** — everything user-visible is `rstack.*`. Legacy `rslint.*` / `rstest.*` settings and command ids are not read, aliased or migrated (breaking old settings and keybindings was an accepted cost).
@@ -23,6 +23,8 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
2323
8. **Self-documenting Rslint diagnostics** — client-side providers parse Inline directives into per-rule hover, DocumentLink and underline-decoration affordances (the hover renders `Rslint(rule-id)`, the shape VS Code gives the published diagnostics), and the router enriches today's `[rule-id] message` diagnostics with a derived Rule docs link. No rule metadata or network lookup is bundled (ADR 0004). The hover provider yields whenever the owning language client's resolved capabilities advertise `hoverProvider`; an optional `Rslint.onClosed` hook identity-safely prunes the controller's capability mirror; the diagnostic synthesis is removed once upstream publishes `code` / `codeDescription` natively.
2424
9. **Color env parity with the CLI** (test) — upstream hard-codes `FORCE_COLOR: '1'` into the worker's spawn env; ours mirrors the CLI's `getForceColorEnv` (rstest `packages/core/src/utils/logger.ts`) instead (`stacks/test/shared/colorEnv.ts`): the master injects `FORCE_COLOR=1` into the composed spawn env only when neither `FORCE_COLOR` nor `NO_COLOR` is already set (marking the injection with `RSTACK_FORCE_COLOR_INJECTED`), and the worker retracts the marked injection right after config load if the config set `NO_COLOR` — the CLI's own decision point. Otherwise a project whose config sets `process.env.NO_COLOR` (rstack-cli does) hits Node's "'NO_COLOR' env is ignored" warning in every pool process. A user-set `FORCE_COLOR` beside a config-set `NO_COLOR` still warns, exactly as the bare CLI does.
2525

26+
10. **Test file ownership** (test) — publication mirrors the CLI per project. When a request carries the same file or the same case from several projects (merged gutter, run-at-cursor, run-current-file; all three profiles reach `startTestRun`), only items of the deepest project root run and the others are reported skipped to clear stale merged gutter state — implemented in `runRouting.ts`, `index.ts` (`startTestRun`), and `master.ts` (`RstestApi.project` made public). Explicit single-project selections, project/folder/Run All, and `rstack.rstest.runInTerminal` (VS Code supplies one explicitly picked item) keep CLI scope.
27+
2628
## Rules
2729

2830
- **Pre-1.0.0 the extension breaks freely.** No compatibility is owed with earlier unpublished states of this extension — settings, command ids and behavior may change without deprecation paths, and dead compat code for them is removed, not kept. No settings migration exists either — not for earlier states of this extension, and not for the two retired standalone extensions (removed in #15; users re-enter their settings under `rstack.*`). Testing and fixtures track only the latest published releases, pinned exactly and bumped by Renovate; a green E2E run speaks only for those releases. `SUPPORT_MATRIX` floors are the minimum versions the extension accepts: each entry is the lowest release evidence shows works with the current code, and its comment records that evidence. Move a floor only when a change makes older releases stop working, never because a devDependency or fixture moved. Raising a floor needs no transition story; the status names the required version.
@@ -41,6 +43,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten
4143

4244
## Gotchas — decisions that look wrong but aren't
4345

46+
- **Why test file ownership exists** (adaptation 10): native configs can overlap upstream too, and the Rstack bridge adds a root contender. VS Code merges same-location items into one gutter action; routing chooses among the request's published items, which already reflect each config's include/exclude rules.
4447
- fmt's `handleShowMessage` suppresses only the classified config-dependency Error. Non-classified `window/showMessage` notifications are re-presented exactly as vscode-languageclient's default handler would (Error/Warning/Info toast). An exact-prefix `rs fmt cannot format this workspace:` Error that is not a missing dependency also reports `crashed` and one Output error line per distinct message; the known config error survives server restarts so polling continues until a format produces edits. This passes through the server's protocol UI request unchanged; "stacks own no UI chrome" constrains UI the stack originates, not protocol UI it relays. Warning and error episodes end on nonempty formatting edits, not merely a response with no new notification: the server returns empty edits on failure and deduplicates showMessage.
4548
- The lint × `rstack.config.*` bridge stays thin on purpose: only a root Rstack config can claim a bridged folder, any native config anywhere in the folder wins ownership, and the worker evaluates rstack's published shim from the folder root. Never generate a shim, load the Rstack config in the extension host, or interpret `define.lint()` ourselves.
4649
- **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.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
26
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import path from 'node:path';
2+
import { defineConfig } from '@rstest/core';
3+
4+
export default defineConfig({
5+
resolve: {
6+
alias: {
7+
'@host-value': path.resolve(__dirname, 'value.ts'),
8+
},
9+
},
10+
});
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import path from 'node:path';
2+
import { expect, test } from '@rstest/core';
3+
import { value } from '@host-value';
4+
5+
test('uses the nested config and cwd', () => {
6+
expect(value).toBe('nested-config');
7+
expect(process.cwd()).toBe(path.resolve(__dirname, '..'));
8+
});
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const value = 'nested-config';
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"name": "rstack-editor-fixture-rstest-ownership",
3+
"version": "0.0.0",
4+
"private": true,
5+
"type": "module",
6+
"dependencies": {
7+
"@rstest/core": "0.11.12",
8+
"rstack": "0.7.4"
9+
}
10+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Independent fixture install; the root serves the nested native project too.
2+
minimumReleaseAgeExclude:
3+
- rstack
4+
- '@rslint/core'
5+
- '@rstest/core'
6+
- '@rsbuild/core'
7+
- '@rslib/core'
8+
- '@rstackjs/*'
9+
- rsbuild-plugin-dts
10+
- '@rspack/*'
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { define } from 'rstack';
2+
3+
// No define.test(): the published shim still supplies default test includes.
4+
define.lint([]);
5+
define.fmt({});

‎packages/vscode/e2e/rstest/suite/bridge.test.ts‎

Lines changed: 165 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
FIXTURES_ROOT,
2525
getRstestExports,
2626
getTestItemByLabels,
27+
getTestItemsRecursive,
2728
toLabelTree,
2829
waitFor,
2930
} from './helpers';
@@ -32,6 +33,24 @@ import {
3233
const RSTACK_FIXTURE = path.resolve(FIXTURES_ROOT, '../../fixtures/rstack');
3334
const RSTACK_FIXTURE_URI = vscode.Uri.file(RSTACK_FIXTURE);
3435

36+
const addFixtureFolder = (uri: vscode.Uri) => {
37+
assert.ok(
38+
vscode.workspace.updateWorkspaceFolders(
39+
vscode.workspace.workspaceFolders?.length || 0,
40+
0,
41+
{ uri },
42+
),
43+
);
44+
};
45+
const removeFixtureFolder = (uri: vscode.Uri) => {
46+
// URI comparison also handles Windows drive-letter casing.
47+
const index = vscode.workspace.workspaceFolders?.findIndex(
48+
(folder) => folder.uri.toString() === uri.toString(),
49+
);
50+
assert.ok(index !== undefined && index >= 0);
51+
assert.ok(vscode.workspace.updateWorkspaceFolders(index, 1));
52+
};
53+
3554
const WORKSPACE_1_FILES = [
3655
{ label: 'each.test.ts' },
3756
{ label: 'foo.test.ts' },
@@ -45,25 +64,11 @@ const WORKSPACE_1_FILES = [
4564
suite('Rstack bridge suite', () => {
4665
suiteSetup(async () => {
4766
await getRstestExports();
48-
const added = vscode.workspace.updateWorkspaceFolders(
49-
vscode.workspace.workspaceFolders?.length || 0,
50-
0,
51-
{ uri: RSTACK_FIXTURE_URI },
52-
);
53-
assert.ok(added, 'adding the rstack fixture folder should be accepted');
67+
addFixtureFolder(RSTACK_FIXTURE_URI);
5468
});
5569

5670
suiteTeardown(async () => {
57-
// Compare `uri.toString()`, not `fsPath`: `fsPath` lower-cases the Windows
58-
// drive letter while `path.resolve` keeps it as-is, so a raw string
59-
// compare can miss on Windows — and a missed removal here would leak the
60-
// folder into every later suite.
61-
const index = vscode.workspace.workspaceFolders?.findIndex(
62-
(folder) => folder.uri.toString() === RSTACK_FIXTURE_URI.toString(),
63-
);
64-
assert.ok(index !== undefined && index >= 0);
65-
const removed = vscode.workspace.updateWorkspaceFolders(index, 1);
66-
assert.ok(removed, 'removing the rstack fixture folder should be accepted');
71+
removeFixtureFolder(RSTACK_FIXTURE_URI);
6772
// Later suites assert on the unwrapped single-folder tree; leave only
6873
// after the controller has actually settled back into it.
6974
await waitFor(() => {
@@ -168,4 +173,148 @@ suite('Rstack bridge suite', () => {
168173
);
169174
assert.match(collecting.output, /1 passed/);
170175
});
176+
177+
test('publishes both projects but routes merged file and case runs to the owner', async () => {
178+
const fixture = path.resolve(
179+
FIXTURES_ROOT,
180+
'../../fixtures/rstest-ownership',
181+
);
182+
const fixtureUri = vscode.Uri.file(fixture);
183+
const testUri = vscode.Uri.file(
184+
path.join(fixture, 'host/tests/ownership.test.ts'),
185+
);
186+
addFixtureFolder(fixtureUri);
187+
const assertOwnership = () => {
188+
const exports = currentRstestExports();
189+
const folder = getTestItemByLabels(exports.testController.items, [
190+
'rstest-ownership',
191+
]);
192+
const root = getTestItemByLabels(folder.children, ['rstack.config.ts']);
193+
const host = getTestItemByLabels(folder.children, ['host']);
194+
// Both configs publish their CLI scope, even where the files overlap.
195+
assert.ok(exports.getResolvedRstestPath(root.id));
196+
assert.ok(exports.getResolvedRstestPath(host.id));
197+
assert.equal(root.busy, false);
198+
assert.equal(host.busy, false);
199+
const files = getTestItemsRecursive(folder.children).filter(
200+
(item) =>
201+
item.uri?.toString() === testUri.toString() &&
202+
item.label === 'ownership.test.ts',
203+
);
204+
const matches = files.map((file) => {
205+
const ancestors: vscode.TestItem[] = [];
206+
for (let parent = file.parent; parent; parent = parent.parent) {
207+
ancestors.unshift(parent);
208+
}
209+
return { file, ancestors };
210+
});
211+
assert.equal(
212+
files.length,
213+
2,
214+
'both projects must publish their own copy of the same URI',
215+
);
216+
const rootFiles = matches.filter(({ ancestors }) =>
217+
ancestors.includes(root),
218+
);
219+
const hostFiles = matches.filter(({ ancestors }) =>
220+
ancestors.includes(host),
221+
);
222+
assert.equal(rootFiles.length, 1, 'expected one file under root');
223+
assert.equal(hostFiles.length, 1, 'expected one file under host');
224+
const rootFile = rootFiles[0].file;
225+
const hostFile = hostFiles[0].file;
226+
assert.equal(rootFile.children.size, 1);
227+
assert.equal(hostFile.children.size, 1);
228+
const rootCase = getTestItemByLabels(rootFile.children, [
229+
'uses the nested config and cwd',
230+
]);
231+
const hostCase = getTestItemByLabels(hostFile.children, [
232+
'uses the nested config and cwd',
233+
]);
234+
return { exports, root, rootFile, hostFile, rootCase, hostCase };
235+
};
236+
const cancellation = new vscode.CancellationTokenSource();
237+
try {
238+
const { exports, root, rootFile, hostFile } = await waitFor(
239+
assertOwnership,
240+
{
241+
timeoutMs: 60_000,
242+
},
243+
);
244+
for (const kind of ['file', 'case']) {
245+
const { rootCase, hostCase } = assertOwnership();
246+
const include =
247+
kind === 'file' ? [rootFile, hostFile] : [rootCase, hostCase];
248+
const collecting = createCollectingMockRun();
249+
await exports.startTestRun(
250+
new vscode.TestRunRequest(include, undefined, exports.runProfile),
251+
cancellation.token,
252+
false,
253+
collecting.createMockRun,
254+
);
255+
await collecting.ended;
256+
assert.ok(
257+
collecting.passedItems.some(
258+
(item) =>
259+
item.parent === hostFile &&
260+
item.label === 'uses the nested config and cwd',
261+
),
262+
);
263+
assert.equal(collecting.failedItems.length, 0);
264+
assert.deepStrictEqual(
265+
collecting.skippedItems,
266+
kind === 'file' ? [rootFile, rootCase] : [rootCase],
267+
);
268+
for (const records of [
269+
collecting.enqueuedItems,
270+
collecting.passedItems,
271+
collecting.failedItems,
272+
]) {
273+
assert.ok(
274+
!records.some(
275+
(item) => item === rootFile || item.parent === rootFile,
276+
),
277+
);
278+
}
279+
}
280+
// Project Run All keeps the root CLI scope, which lacks the host alias.
281+
const rootRun = createCollectingMockRun();
282+
await exports.startTestRun(
283+
new vscode.TestRunRequest([root], undefined, exports.runProfile),
284+
cancellation.token,
285+
false,
286+
rootRun.createMockRun,
287+
);
288+
await rootRun.ended;
289+
assert.ok(rootRun.failedItems.includes(rootFile));
290+
assert.match(
291+
rootRun.failedMessages
292+
.map((message) => String(message.message))
293+
.join('\n'),
294+
/@host-value/,
295+
);
296+
for (const records of [
297+
rootRun.enqueuedItems,
298+
rootRun.passedItems,
299+
rootRun.failedItems,
300+
]) {
301+
assert.ok(
302+
!records.some(
303+
(item) => item === hostFile || item.parent === hostFile,
304+
),
305+
);
306+
}
307+
} finally {
308+
cancellation.dispose();
309+
removeFixtureFolder(fixtureUri);
310+
await waitFor(() => {
311+
assert.equal(currentRstestExports().testController.items.size, 2);
312+
assert.ok(
313+
!vscode.workspace.workspaceFolders?.some(
314+
(folder) => folder.uri.toString() === fixtureUri.toString(),
315+
),
316+
);
317+
});
318+
}
319+
});
171320
});

0 commit comments

Comments
 (0)