fix(run): preserve pnpm test selection arguments (#2092)

src/features/run/configs.test.ts

@@ -193,13 +193,32 @@ describe("context configuration", () => {
   it("runs a JS test file through the closest test script", () => {
     const c = contextConfig("web/src/cart.test.ts", configs)!;
     expect(c).toMatchObject({
-      args: ["run", "test", "--", "src/cart.test.ts"],
+      args: ["run", "test", "src/cart.test.ts"],
       cwd: "web",
       isTest: true,
       name: "cart.test.ts",
     });
     expect(c.id).toBe("context:npm:web:test:web/src/cart.test.ts");
-    expect(contextConfig("src/a.spec.js", configs)!.args).toEqual(["run", "test", "--", "src/a.spec.js"]);
+    expect(contextConfig("src/a.spec.js", configs)!.args).toEqual(["run", "test", "src/a.spec.js"]);
+  });
+
+  it.each([
+    ["npm", ["--"]],
+    ["pnpm", []],
+    ["/opt/bin/pnpm", []],
+    ["C:\\tools\\pnpm.CMD", []],
+    ["yarn", ["--"]],
+    ["bun", ["--"]],
+  ])("forwards test files through %s", (command, separator) => {
+    const base = cfg({ id: "js:test", kind: "npm", command, args: ["run", "test"], isTest: true });
+    expect(contextConfig("tests/cart.test.ts", [base])!.args).toEqual([
+      "run",
+      "test",
+      ...separator,
+      "tests/cart.test.ts",
+    ]);
+    const explicit = { ...base, args: ["run", "test", "--", "--reporter=verbose"] };
+    expect(contextConfig("tests/cart.test.ts", [explicit])!.args).toEqual([...explicit.args, "tests/cart.test.ts"]);
   });
 
   it("narrows rspec and rake test runs to the file", () => {

src/features/run/context.ts

@@ -11,6 +11,7 @@ import { goBuildType } from "./goTypes";
 import { isWindows } from "./jvmTypes";
 import { typeFor } from "./types";
 import { runTemplate } from "./runTemplates";
+import { appendScriptArgs } from "./scriptArgs";
 
 export type FileRole = "test" | "main" | "script" | null;
 
@@ -216,8 +217,7 @@ export function testConfigFor(relPath: string, configs: RunConfig[], base?: RunC
   switch (base.kind) {
     case "npm":
     case "bun": {
-      // `npm run test -- file` works for npm, pnpm, yarn and bun.
-      const args = base.args.includes("--") ? [...base.args, file] : [...base.args, "--", file];
+      const args = appendScriptArgs(base.command, base.args, [file]);
       return derived(base, relPath, { args });
     }
     case "deno":

src/features/run/scriptArgs.ts

@@ -0,0 +1,7 @@
+import { commandName } from "./commandName";
+
+/** pnpm forwards `--` to the script, where it ends the test runner's option parsing. */
+export function appendScriptArgs(command: string, args: string[], extra: string[]): string[] {
+  const separator = !args.includes("--") && ["npm", "yarn", "bun"].includes(commandName(command)) ? ["--"] : [];
+  return [...args, ...separator, ...extra];
+}

src/features/run/targets.test.ts

@@ -170,7 +170,6 @@ describe("targetConfig", () => {
     expect(first("describe('m', () => {\n  it('adds', () => {});\n});", "src/a.test.ts").args).toEqual([
       "run",
       "test",
-      "--",
       "src/a.test.ts",
       "-t",
       "^m adds$",

src/features/run/testing/rerun.test.ts

@@ -86,15 +86,23 @@ describe("rerunFailedConfig", () => {
     expect(c.args).toEqual(["test", "--", "--nocapture", "old", "a", "--exact"]);
   });
 
-  it("jest/vitest via npm: files + -t pattern", () => {
-    const c = rerunFailedConfig(cfg("npm", "pnpm", ["run", "test"]), [
+  it.each([
+    ["npm", ["--"]],
+    ["pnpm", []],
+    ["/opt/bin/pnpm", []],
+    ["C:\\tools\\pnpm.CMD", []],
+    ["yarn", ["--"]],
+    ["bun", ["--"]],
+    ["npx", []],
+  ])("jest/vitest via %s: files + -t pattern", (command, separator) => {
+    const c = rerunFailedConfig(cfg("npm", command, ["run", "test"]), [
       ["src/a.test.ts", "math", "adds (1+1)"],
       ["src/b.test.ts"],
     ])!;
     expect(c.args).toEqual([
       "run",
       "test",
-      "--",
+      ...separator,
       "src/a.test.ts",
       "src/b.test.ts",
       "-t",
@@ -102,6 +110,14 @@ describe("rerunFailedConfig", () => {
     ]);
   });
 
+  it.each([
+    ["run", "test", "-t", "old"],
+    ["run", "test", "--", "--testNamePattern=old"],
+  ])("replaces an existing pnpm test filter in %j", (...args) => {
+    const c = rerunFailedConfig(cfg("npm", "pnpm", args), [["src/a.test.ts", "new"]])!;
+    expect(c.args).toEqual(["run", "test", ...(args.includes("--") ? ["--"] : []), "src/a.test.ts", "-t", "^(?:new)$"]);
+  });
+
   it("pytest: node ids replace explicit paths", () => {
     const c = rerunFailedConfig(cfg("pytest", ".venv/bin/pytest", ["-q", "tests/", "-k", "slow"]), [
       ["tests/test_x.py", "TestC", "test_y[1]"],

src/features/run/testing/rerun.ts

@@ -5,6 +5,7 @@
 import { phpunitArgs, phpunitFilter } from "./phpunit";
 import type { RunConfig } from "@/platform/ipc";
 import { formatArgs } from "../configs";
+import { appendScriptArgs } from "../scriptArgs";
 import { djangoTestsOf } from "../djangoTypes";
 import { goTestParts, withoutGoSelection } from "../goArguments";
 import { frameworkFor } from "./index";
@@ -71,11 +72,10 @@ function jsArgs(config: RunConfig, paths: string[][]): string[] | null {
   const names = unique(paths.filter((p) => p.length > 1).map((p) => p.slice(1).join(" ")));
   if (!files.length) return null;
   const { before, after } = splitDashDash(base);
-  const rest = dropFlag(after ?? [], ["-t", "--testNamePattern"]);
+  const flags = ["-t", "--testNamePattern"];
+  const args = after === null ? dropFlag(before, flags) : [...before, "--", ...dropFlag(after, flags)];
   const filter = names.length ? ["-t", `^(?:${names.map(escapeRegExp).join("|")})$`] : [];
-  // npm/pnpm/yarn/bun pass everything after `--` to the script; deno/npx take them directly.
-  const needsDashDash = after !== null || /(^|\/)(npm|pnpm|yarn|bun)$/.test(config.command);
-  return needsDashDash ? [...before, "--", ...rest, ...files, ...filter] : [...before, ...files, ...filter];
+  return appendScriptArgs(config.command, args, [...files, ...filter]);
 }
 
 function pytestArgs(base: string[], paths: string[][]): string[] | null {