-
Notifications
You must be signed in to change notification settings - Fork 3
test: add plugin routing tests for bundled irssi plugin #352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,73 @@ | ||||||
| const fs = require("fs") | ||||||
| const os = require("os") | ||||||
| const path = require("path") | ||||||
| const { execSync } = require("child_process") | ||||||
|
|
||||||
| const CLI = path.join(__dirname, "..", "cli", "supercli.js") | ||||||
|
|
||||||
| function runNoServer(args, options = {}) { | ||||||
| try { | ||||||
| const env = { ...process.env } | ||||||
| delete env.SUPERCLI_SERVER | ||||||
| const out = execSync(`node ${CLI} ${args}`, { | ||||||
| encoding: "utf-8", | ||||||
| timeout: 15000, | ||||||
| env: { ...env, ...(options.env || {}) } | ||||||
| }) | ||||||
| return { ok: true, output: out.trim(), code: 0 } | ||||||
| } catch (err) { | ||||||
| return { | ||||||
| ok: false, | ||||||
| output: (err.stdout || "").trim(), | ||||||
| stderr: (err.stderr || "").trim(), | ||||||
| code: err.status | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| function writeFakeDarBinary(dir) { | ||||||
| const bin = path.join(dir, "dar") | ||||||
| fs.writeFileSync(bin, [ | ||||||
| "#!/usr/bin/env node", | ||||||
| "const args = process.argv.slice(2);", | ||||||
| "if (args.includes('--version')) { console.log('dar 2.7.13-test'); process.exit(0); }", | ||||||
| "console.log(JSON.stringify({ ok: true, args }));" | ||||||
| ].join("\n"), "utf-8") | ||||||
| fs.chmodSync(bin, 0o755) | ||||||
| return bin | ||||||
| } | ||||||
|
|
||||||
| describe("dar plugin", () => { | ||||||
| const fakeDir = fs.mkdtempSync(path.join(os.tmpdir(), "dcli-dar-")) | ||||||
| const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "dcli-home-dar-")) | ||||||
| writeFakeDarBinary(fakeDir) | ||||||
| const env = { ...process.env, PATH: `${fakeDir}:${process.env.PATH || ""}`, SUPERCLI_HOME: tempHome } | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Use Hardcoding
💻 Proposed fix- const env = { ...process.env, PATH: `${fakeDir}:${process.env.PATH || ""}`, SUPERCLI_HOME: tempHome }
+ const env = { ...process.env, PATH: `${fakeDir}${path.delimiter}${process.env.PATH || ""}`, SUPERCLI_HOME: tempHome }📝 Committable suggestion
Suggested change
📍 Affects 3 files
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| beforeAll(() => { | ||||||
| const install = runNoServer("plugins install ./plugins/dar --on-conflict replace --json", { env }) | ||||||
| expect(install.ok).toBe(true) | ||||||
| }) | ||||||
|
|
||||||
| afterAll(() => { | ||||||
| runNoServer("plugins remove dar --json", { env }) | ||||||
| fs.rmSync(fakeDir, { recursive: true, force: true }) | ||||||
| fs.rmSync(tempHome, { recursive: true, force: true }) | ||||||
| }) | ||||||
|
|
||||||
| test("supports namespace passthrough", () => { | ||||||
| const r = runNoServer("dar --create backup --json", { env }) | ||||||
| expect(r.ok).toBe(true) | ||||||
| const data = JSON.parse(r.output) | ||||||
| expect(data.command).toBe("dar.passthrough") | ||||||
| expect(data.data.args).toContain("--create") | ||||||
| expect(data.data.args).toContain("backup") | ||||||
| }) | ||||||
|
|
||||||
| test("doctor reports dar dependency as healthy", () => { | ||||||
| const r = runNoServer("plugins doctor dar --json", { env }) | ||||||
| expect(r.ok).toBe(true) | ||||||
| const data = JSON.parse(r.output) | ||||||
| expect(data.ok).toBe(true) | ||||||
| expect(data.checks.some(c => c.type === "binary" && c.binary === "dar" && c.ok === true)).toBe(true) | ||||||
| }) | ||||||
| }) | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| const fs = require("fs") | ||
| const os = require("os") | ||
| const path = require("path") | ||
| const { execSync } = require("child_process") | ||
|
|
||
| const CLI = path.join(__dirname, "..", "cli", "supercli.js") | ||
|
|
||
| function runNoServer(args, options = {}) { | ||
| try { | ||
| const env = { ...process.env } | ||
| delete env.SUPERCLI_SERVER | ||
| const out = execSync(`node ${CLI} ${args}`, { | ||
| encoding: "utf-8", | ||
| timeout: 15000, | ||
| env: { ...env, ...(options.env || {}) } | ||
| }) | ||
| return { ok: true, output: out.trim(), code: 0 } | ||
| } catch (err) { | ||
| return { | ||
| ok: false, | ||
| output: (err.stdout || "").trim(), | ||
| stderr: (err.stderr || "").trim(), | ||
| code: err.status | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function writeFakeIrssiBinary(dir) { | ||
| const bin = path.join(dir, "irssi") | ||
| fs.writeFileSync(bin, [ | ||
| "#!/usr/bin/env node", | ||
| "const args = process.argv.slice(2);", | ||
| "if (args.includes('--version')) { console.log('irssi 1.4.5-test'); process.exit(0); }", | ||
| "console.log(JSON.stringify({ ok: true, args }));" | ||
| ].join("\n"), "utf-8") | ||
| fs.chmodSync(bin, 0o755) | ||
| return bin | ||
| } | ||
|
|
||
| describe("irssi plugin", () => { | ||
| const fakeDir = fs.mkdtempSync(path.join(os.tmpdir(), "dcli-irssi-")) | ||
| const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "dcli-home-irssi-")) | ||
| writeFakeIrssiBinary(fakeDir) | ||
| const env = { ...process.env, PATH: `${fakeDir}:${process.env.PATH || ""}`, SUPERCLI_HOME: tempHome } | ||
|
|
||
| beforeAll(() => { | ||
| const install = runNoServer("plugins install ./plugins/irssi --on-conflict replace --json", { env }) | ||
| expect(install.ok).toBe(true) | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| runNoServer("plugins remove irssi --json", { env }) | ||
| fs.rmSync(fakeDir, { recursive: true, force: true }) | ||
| fs.rmSync(tempHome, { recursive: true, force: true }) | ||
| }) | ||
|
|
||
| test("supports namespace passthrough", () => { | ||
| const r = runNoServer("irssi connect irc.libera.chat --json", { env }) | ||
| expect(r.ok).toBe(true) | ||
| const data = JSON.parse(r.output) | ||
| expect(data.command).toBe("irssi.passthrough") | ||
| expect(data.data.args).toContain("connect") | ||
| expect(data.data.args).toContain("irc.libera.chat") | ||
| }) | ||
|
|
||
| test("doctor reports irssi dependency as healthy", () => { | ||
| const r = runNoServer("plugins doctor irssi --json", { env }) | ||
| expect(r.ok).toBe(true) | ||
| const data = JSON.parse(r.output) | ||
| expect(data.ok).toBe(true) | ||
| expect(data.checks.some(c => c.type === "binary" && c.binary === "irssi" && c.ok === true)).toBe(true) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| const fs = require("fs") | ||
| const os = require("os") | ||
| const path = require("path") | ||
| const { execSync } = require("child_process") | ||
|
|
||
| const CLI = path.join(__dirname, "..", "cli", "supercli.js") | ||
|
|
||
| function runNoServer(args, options = {}) { | ||
| try { | ||
| const env = { ...process.env } | ||
| delete env.SUPERCLI_SERVER | ||
| const out = execSync(`node ${CLI} ${args}`, { | ||
| encoding: "utf-8", | ||
| timeout: 15000, | ||
| env: { ...env, ...(options.env || {}) } | ||
| }) | ||
| return { ok: true, output: out.trim(), code: 0 } | ||
| } catch (err) { | ||
| return { | ||
| ok: false, | ||
| output: (err.stdout || "").trim(), | ||
| stderr: (err.stderr || "").trim(), | ||
| code: err.status | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function writeFakeXataBinary(dir) { | ||
| const bin = path.join(dir, "xata") | ||
| fs.writeFileSync(bin, [ | ||
| "#!/usr/bin/env node", | ||
| "const args = process.argv.slice(2);", | ||
| "if (args.includes('--version')) { console.log('xata 0.16.0-test'); process.exit(0); }", | ||
| "console.log(JSON.stringify({ ok: true, args }));" | ||
| ].join("\n"), "utf-8") | ||
| fs.chmodSync(bin, 0o755) | ||
| return bin | ||
| } | ||
|
|
||
| describe("xata plugin", () => { | ||
| const fakeDir = fs.mkdtempSync(path.join(os.tmpdir(), "dcli-xata-")) | ||
| const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "dcli-home-xata-")) | ||
| writeFakeXataBinary(fakeDir) | ||
| const env = { ...process.env, PATH: `${fakeDir}:${process.env.PATH || ""}`, SUPERCLI_HOME: tempHome } | ||
|
|
||
| beforeAll(() => { | ||
| const install = runNoServer("plugins install ./plugins/xata --on-conflict replace --json", { env }) | ||
| expect(install.ok).toBe(true) | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| runNoServer("plugins remove xata --json", { env }) | ||
| fs.rmSync(fakeDir, { recursive: true, force: true }) | ||
| fs.rmSync(tempHome, { recursive: true, force: true }) | ||
| }) | ||
|
|
||
| test("supports namespace passthrough", () => { | ||
| const r = runNoServer("xata branch list --json", { env }) | ||
| expect(r.ok).toBe(true) | ||
| const data = JSON.parse(r.output) | ||
| expect(data.command).toBe("xata.passthrough") | ||
| expect(data.data.args).toContain("branch") | ||
| expect(data.data.args).toContain("list") | ||
| }) | ||
|
|
||
| test("doctor reports xata dependency as healthy", () => { | ||
| const r = runNoServer("plugins doctor xata --json", { env }) | ||
| expect(r.ok).toBe(true) | ||
| const data = JSON.parse(r.output) | ||
| expect(data.ok).toBe(true) | ||
| expect(data.checks.some(c => c.type === "binary" && c.binary === "xata" && c.ok === true)).toBe(true) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Quote the CLI path to prevent execution failures.
If the repository is cloned into a directory with spaces in its name,
execSyncwill fail because the shell will improperly split the unquoted${CLI}path. Wrap${CLI}in quotes to ensure it resolves correctly.__tests__/dar-plugin.test.js#L12-L16: change`node ${CLI} ${args}`to`node "${CLI}" ${args}`.__tests__/irssi-plugin.test.js#L12-L16: apply the same quoting to theexecSynccall.__tests__/xata-plugin.test.js#L12-L16: apply the same quoting to theexecSynccall.💻 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 OpenGrep (1.25.0)
[ERROR] 12-16: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
📍 Affects 3 files
__tests__/dar-plugin.test.js#L12-L16(this comment)__tests__/irssi-plugin.test.js#L12-L16__tests__/xata-plugin.test.js#L12-L16🤖 Prompt for AI Agents