Skip to content

Commit

Permalink
fix(runtime/js/spawn): Pass stdio options for spawn() and spawnSync() (
Browse files Browse the repository at this point in the history
  • Loading branch information
nayeemrmn authored Apr 24, 2022
1 parent 4b7d306 commit e9041b9
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 53 deletions.
4 changes: 2 additions & 2 deletions cli/dts/lib.deno.unstable.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,7 @@ declare namespace Deno {
/**
* Executes a subprocess, waiting for it to finish and
* collecting all of its output.
* The stdio options are ignored.
* Will throw an error if `stdin: "piped"` is passed.
*
* ```ts
* const { status, stdout, stderr } = await Deno.spawn(Deno.execPath(), {
Expand All @@ -1464,7 +1464,7 @@ declare namespace Deno {
/**
* Synchronously executes a subprocess, waiting for it to finish and
* collecting all of its output.
* The stdio options are ignored.
* Will throw an error if `stdin: "piped"` is passed.
*
* ```ts
* const { status, stdout, stderr } = Deno.spawnSync(Deno.execPath(), {
Expand Down
5 changes: 5 additions & 0 deletions cli/tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2764,3 +2764,8 @@ itest!(report_error_handled {
args: "run --quiet report_error_handled.ts",
output: "report_error_handled.ts.out",
});

itest!(spawn_stdout_inherit {
args: "run --quiet --unstable -A spawn_stdout_inherit.ts",
output: "spawn_stdout_inherit.ts.out",
});
8 changes: 8 additions & 0 deletions cli/tests/testdata/spawn_stdout_inherit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
await Deno.spawn(Deno.execPath(), {
args: ["eval", "--quiet", "console.log('Hello, world! 1')"],
stdout: "inherit",
});
Deno.spawnSync(Deno.execPath(), {
args: ["eval", "--quiet", "console.log('Hello, world! 2')"],
stdout: "inherit",
});
2 changes: 2 additions & 0 deletions cli/tests/testdata/spawn_stdout_inherit.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Hello, world! 1
Hello, world! 2
62 changes: 22 additions & 40 deletions cli/tests/unit/command_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,46 +468,6 @@ Deno.test(
},
);

Deno.test(
{ permissions: { run: true, read: true } },
async function spawnOverrideStdio() {
const { stdout, stderr } = await Deno.spawn(Deno.execPath(), {
args: [
"eval",
"console.log('hello'); console.error('world')",
],
stdin: "piped",
stdout: "null",
stderr: "null",
});

// @ts-ignore: for testing
assertEquals(new TextDecoder().decode(stdout), "hello\n");
// @ts-ignore: for testing
assertEquals(new TextDecoder().decode(stderr), "world\n");
},
);

Deno.test(
{ permissions: { run: true, read: true } },
function spawnSyncOverrideStdio() {
const { stdout, stderr } = Deno.spawnSync(Deno.execPath(), {
args: [
"eval",
"console.log('hello'); console.error('world')",
],
stdin: "piped",
stdout: "null",
stderr: "null",
});

// @ts-ignore: for testing
assertEquals(new TextDecoder().decode(stdout), "hello\n");
// @ts-ignore: for testing
assertEquals(new TextDecoder().decode(stderr), "world\n");
},
);

Deno.test(
{ permissions: { run: true, read: true } },
async function spawnEnv() {
Expand Down Expand Up @@ -685,3 +645,25 @@ Deno.test(
}
},
);

Deno.test(async function spawnStdinPipedFails() {
await assertRejects(
() =>
Deno.spawn("id", {
stdin: "piped",
}),
TypeError,
"Piped stdin is not supported for this function, use 'Deno.spawnChild()' instead",
);
});

Deno.test(function spawnSyncStdinPipedFails() {
assertThrows(
() =>
Deno.spawnSync("id", {
stdin: "piped",
}),
TypeError,
"Piped stdin is not supported for this function, use 'Deno.spawnChild()' instead",
);
});
30 changes: 19 additions & 11 deletions runtime/js/40_spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,13 @@
}
}

function spawn(command, options) { // TODO(@crowlKats): more options (like input)?
return spawnChild(command, {
...options,
stdin: "null",
stdout: "piped",
stderr: "piped",
}).output();
function spawn(command, options) {
if (options?.stdin === "piped") {
throw new TypeError(
"Piped stdin is not supported for this function, use 'Deno.spawnChild()' instead",
);
}
return spawnChild(command, options).output();
}

function spawnSync(command, {
Expand All @@ -182,7 +182,15 @@
env = {},
uid = undefined,
gid = undefined,
} = {}) { // TODO(@crowlKats): more options (like input)?
stdin = "null",
stdout = "piped",
stderr = "piped",
} = {}) {
if (stdin === "piped") {
throw new TypeError(
"Piped stdin is not supported for this function, use 'Deno.spawnChild()' instead",
);
}
return core.opSync("op_spawn_sync", {
cmd: pathFromURL(command),
args: ArrayPrototypeMap(args, String),
Expand All @@ -191,9 +199,9 @@
env: ObjectEntries(env),
uid,
gid,
stdin: "null",
stdout: "piped",
stderr: "piped",
stdin,
stdout,
stderr,
});
}

Expand Down

0 comments on commit e9041b9

Please sign in to comment.