From bb51695eeafa7126ef04d51a5a2ac05a20f3b7a8 Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Wed, 11 Oct 2023 00:11:08 +0200 Subject: [PATCH] Improve Stubscape Update the Stubscape implementation so that the behavior of `quote` and `quoteAll` is more similar to the real implementation of Shescape. Tests are updated accordingly, mostly resulting in cleaner integration tests (with more coverage) at the minor cost of more specific unit tests (that actually more accurately reflect expected behavior). Also update the Stubscape JSDoc to be up-to-date, both with respect to the changes seen here as well as the changes of v3.0.0. --- test/integration/testing/commonjs.test.js | 34 ++++++++++++++-- test/integration/testing/functional.test.js | 10 +---- test/unit/testing/stubscape.test.js | 44 +++++++++++++++++++-- testing.js | 20 +++++++--- 4 files changed, 87 insertions(+), 21 deletions(-) diff --git a/test/integration/testing/commonjs.test.js b/test/integration/testing/commonjs.test.js index 707c73b92..a0576fd24 100644 --- a/test/integration/testing/commonjs.test.js +++ b/test/integration/testing/commonjs.test.js @@ -45,11 +45,24 @@ testProp( "Stubscape#quote (esm === cjs)", [arbitrary.shescapeArg(), arbitrary.shescapeOptions()], (t, arg, options) => { + let resultEsm, resultCjs, erroredEsm, erroredCjs; + const stubscape = new Stubscape(options); const stubscapeCjs = new StubscapeCjs(options); - const resultEsm = stubscape.quote(arg); - const resultCjs = stubscapeCjs.quote(arg); + try { + resultEsm = stubscape.quote(arg); + } catch (_) { + erroredEsm = true; + } + + try { + resultCjs = stubscapeCjs.quote(arg); + } catch (_) { + erroredCjs = true; + } + + t.is(erroredEsm, erroredCjs); t.is(resultEsm, resultCjs); }, ); @@ -58,11 +71,24 @@ testProp( "Stubscape#quoteAll (esm === cjs)", [fc.array(arbitrary.shescapeArg()), arbitrary.shescapeOptions()], (t, args, options) => { + let resultEsm, resultCjs, erroredEsm, erroredCjs; + const stubscape = new Stubscape(options); const stubscapeCjs = new StubscapeCjs(options); - const resultEsm = stubscape.quoteAll(args); - const resultCjs = stubscapeCjs.quoteAll(args); + try { + resultEsm = stubscape.quoteAll(args); + } catch (_) { + erroredEsm = true; + } + + try { + resultCjs = stubscapeCjs.quoteAll(args); + } catch (_) { + erroredCjs = true; + } + + t.is(erroredEsm, erroredCjs); t.deepEqual(resultEsm, resultCjs); }, ); diff --git a/test/integration/testing/functional.test.js b/test/integration/testing/functional.test.js index f307b3323..ec84f75db 100644 --- a/test/integration/testing/functional.test.js +++ b/test/integration/testing/functional.test.js @@ -78,10 +78,7 @@ testProp( testProp( "Stubscape#quote, with shell (stubscape =~ shescape)", - [ - fc.anything(), - arbitrary.shescapeOptions().filter((options) => options?.shell !== false), - ], + [fc.anything(), arbitrary.shescapeOptions()], (t, arg, options) => { let result, stubResult, errored, stubErrored; @@ -113,10 +110,7 @@ testProp( testProp( "Stubscape#quoteAll, with shell (stubscape =~ shescape)", - [ - fc.anything(), - arbitrary.shescapeOptions().filter((options) => options?.shell !== false), - ], + [fc.anything(), arbitrary.shescapeOptions()], (t, args, options) => { let result, stubResult, errored, stubErrored; diff --git a/test/unit/testing/stubscape.test.js b/test/unit/testing/stubscape.test.js index 5c85ac164..d5f1bc8cf 100644 --- a/test/unit/testing/stubscape.test.js +++ b/test/unit/testing/stubscape.test.js @@ -68,7 +68,10 @@ test("escapeAll invalid arguments", (t) => { testProp( "quote valid arguments", - [arbitrary.shescapeArg(), arbitrary.shescapeOptions()], + [ + arbitrary.shescapeArg(), + arbitrary.shescapeOptions().filter((options) => options?.shell !== false), + ], (t, arg, options) => { const stubscape = new Stubscape(options); const result = stubscape.quote(arg); @@ -87,9 +90,26 @@ test("quote invalid arguments", (t) => { } }); +testProp( + "quote without a shell", + [ + arbitrary.shescapeArg(), + arbitrary.shescapeOptions().filter((options) => options?.shell === false), + ], + (t, arg, options) => { + const stubscape = new Stubscape(options); + t.throws(() => stubscape.quote(arg), { + instanceOf: Error, + }); + }, +); + testProp( "quoteAll valid arguments", - [fc.array(arbitrary.shescapeArg()), arbitrary.shescapeOptions()], + [ + fc.array(arbitrary.shescapeArg()), + arbitrary.shescapeOptions().filter((options) => options?.shell !== false), + ], (t, args, options) => { const stubscape = new Stubscape(options); const result = stubscape.quoteAll(args); @@ -100,12 +120,14 @@ testProp( testProp( "quoteAll non-array arguments", - [arbitrary.shescapeArg(), arbitrary.shescapeOptions()], + [ + arbitrary.shescapeArg(), + arbitrary.shescapeOptions().filter((options) => options?.shell !== false), + ], (t, arg, options) => { const stubscape = new Stubscape(options); t.throws(() => stubscape.quoteAll(arg), { instanceOf: TypeError, - message: "args.map is not a function", }); }, ); @@ -120,3 +142,17 @@ test("quoteAll invalid arguments", (t) => { }); } }); + +testProp( + "quoteAll without a shell", + [ + fc.array(arbitrary.shescapeArg()), + arbitrary.shescapeOptions().filter((options) => options?.shell === false), + ], + (t, args, options) => { + const stubscape = new Stubscape(options); + t.throws(() => stubscape.quoteAll(args), { + instanceOf: Error, + }); + }, +); diff --git a/testing.js b/testing.js index 97352d4bf..9e999149a 100644 --- a/testing.js +++ b/testing.js @@ -26,17 +26,19 @@ export const injectionStrings = [ ]; /** - * A test stub of Shescape that has the same input-output profile as the real - * Shescape implementation. + * An optimistic test stub of Shescape that has the same input-output profile as + * the real Shescape implementation. * * In particular: + * - The constructor never fails. * - Returns a string for all stringable inputs. * - Errors on non-stringable inputs. - * - Converts non-array inputs to single-item arrays where necessary. + * - Errors on non-array inputs where arrays are expected. + * - Errors when trying to quote when `shell: false`. */ export class Shescape { - constructor(_options) { - // Nothing to do. + constructor(options = {}) { + this.shell = options.shell; } escape(arg) { @@ -48,10 +50,18 @@ export class Shescape { } quote(arg) { + if (this.shell === false) { + throw new Error(); + } + return this.escape(arg); } quoteAll(args) { + if (this.shell === false) { + throw new Error(); + } + return this.escapeAll(args); } }