From e9c6325152baa5af01972b3b14e2d71a6b0f353a Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Thu, 20 Oct 2022 22:52:39 +0200 Subject: [PATCH 1/8] feat: set exit code depending on what went wrong --- bin/cli.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index 192963a..35f1ea5 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -24,7 +24,7 @@ console.error = logger.error.bind(logger); // Pretty logs all errors, then exits process.on("uncaughtException", err => { logger.error(err); - process.exit(1); + process.exit(2); }); // Generates the CLI binding using meow @@ -72,12 +72,12 @@ process.on("exit", () => { logger.debug("No configuration file found"); if (cli.flags.config) { logger.error("Configuration file not found"); - process.exit(1); + process.exit(3); } } } catch (e) { logger.error(`Failed to parse config: ${e.stack}`); - process.exit(1); + process.exit(4); } // lint all the files From df364ac73e182f12cc4ba7a2894787279fe48e7e Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Thu, 20 Oct 2022 23:33:42 +0200 Subject: [PATCH 2/8] test: exit codes related to configuration files --- test/cli.spec.js | 16 ++++++++++++++++ test/configs/invalid-svglint-config.js | 1 + 2 files changed, 17 insertions(+) create mode 100644 test/configs/invalid-svglint-config.js diff --git a/test/cli.spec.js b/test/cli.spec.js index e052085..1f6820b 100644 --- a/test/cli.spec.js +++ b/test/cli.spec.js @@ -43,4 +43,20 @@ describe("CLI", function(){ expect(failed).toBeTruthy(); expect(exitCode).toBe(1); }); + + it("should fail with an non-existent configuration file", async function(){ + const { failed, exitCode } = await execCliWith([ + "--config", "./this/file/does/not-exist.js" + ]); + expect(failed).toBeTruthy(); + expect(exitCode).toBe(3); + }); + + it("should fail with an invalid configuration file", async function(){ + const { failed, exitCode } = await execCliWith([ + "--config", "./test/configs/invalid-svglint-config.js" + ]); + expect(failed).toBeTruthy(); + expect(exitCode).toBe(4); + }); }); diff --git a/test/configs/invalid-svglint-config.js b/test/configs/invalid-svglint-config.js new file mode 100644 index 0000000..16ce402 --- /dev/null +++ b/test/configs/invalid-svglint-config.js @@ -0,0 +1 @@ +garble garble I'm invalid \ No newline at end of file From ff060dd5663e36ba7ec7d86b2a5b6b707f12a6f3 Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Thu, 20 Oct 2022 23:44:03 +0200 Subject: [PATCH 3/8] chore: don't lint configuration files for tests --- .eslintrc.cjs | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 6007484..7cb6054 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -24,5 +24,8 @@ module.exports = { "always" ], "no-console": "warn", - } + }, + "ignorePatterns": [ + "test/configs/*.js" + ] }; diff --git a/package.json b/package.json index 8dcfa85..e8f79cb 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "author": "birjolaxew", "license": "MIT", "scripts": { - "lint": "eslint bin/cli.js src/**/*.js test/**/*.js", + "lint": "eslint .", "prepublishOnly": "rollup -c", "test": "mocha" }, From a65f3cefcf4df1b2366d7b415a6c7af4ee23b89c Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Sat, 22 Oct 2022 16:45:41 +0200 Subject: [PATCH 4/8] test: align invalid config test with ESM/CJS config tests Use the "projects" directory to create a broken project with an invalid configuration file. --- .eslintrc.yaml | 2 +- test/cli.spec.js | 6 +++--- test/{configs => projects/broken}/invalid-svglint-config.js | 0 3 files changed, 4 insertions(+), 4 deletions(-) rename test/{configs => projects/broken}/invalid-svglint-config.js (100%) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 598852e..5d95aa9 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -22,4 +22,4 @@ rules: - always no-console: warn ignorePatterns: - - test/configs/*.js + - test/projects/broken/*.js diff --git a/test/cli.spec.js b/test/cli.spec.js index 4872a79..3cb964d 100644 --- a/test/cli.spec.js +++ b/test/cli.spec.js @@ -51,7 +51,9 @@ describe("CLI", function(){ expect(failed).toBeTruthy(); expect(exitCode).toBe(1); }); +}); +describe("Configuration files", function() { it("should fail with an non-existent configuration file", async function(){ const { failed, exitCode } = await execCliWith([ "--config", "./this/file/does/not-exist.js" @@ -62,14 +64,12 @@ describe("CLI", function(){ it("should fail with an invalid configuration file", async function(){ const { failed, exitCode } = await execCliWith([ - "--config", "./test/configs/invalid-svglint-config.js" + "--config", "./test/projects/broken/invalid-svglint-config.js" ]); expect(failed).toBeTruthy(); expect(exitCode).toBe(4); }); -}); -describe("Configuration files", function() { it("should fail passing an non-existent file path to --config", async function() { const { failed, exitCode } = await execCliWith( [VALID_SVG, "--config", "./this/file/does/not-exist.js"], diff --git a/test/configs/invalid-svglint-config.js b/test/projects/broken/invalid-svglint-config.js similarity index 100% rename from test/configs/invalid-svglint-config.js rename to test/projects/broken/invalid-svglint-config.js From ee3f9a9e9b7c5317341f6b4b52b7a8e7e5575664 Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Sat, 22 Oct 2022 16:47:03 +0200 Subject: [PATCH 5/8] test: Remove duplicate test --- test/cli.spec.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/cli.spec.js b/test/cli.spec.js index 3cb964d..f809b70 100644 --- a/test/cli.spec.js +++ b/test/cli.spec.js @@ -70,14 +70,6 @@ describe("Configuration files", function() { expect(exitCode).toBe(4); }); - it("should fail passing an non-existent file path to --config", async function() { - const { failed, exitCode } = await execCliWith( - [VALID_SVG, "--config", "./this/file/does/not-exist.js"], - ); - expect(failed).toBeTruthy(); - expect(exitCode).toBe(1); - }); - it("should succeed passing an existent file path to --config", async function() { const { failed } = await execCliWith( [VALID_SVG, "--config", "test/projects/esm/foo/custom-svglint-config.js"] From a0088958c07eb8afda9cdd28d17004fff137ef84 Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Sat, 22 Oct 2022 16:48:13 +0200 Subject: [PATCH 6/8] chore: rename invalid config to broken config Invalid could mean that the configuration is invalid for SVGLint, but it's supposed to mean that it's invalid JavaScript. Broken seems clearer while being equally brief. --- test/cli.spec.js | 4 ++-- .../{invalid-svglint-config.js => broken-svglint-config.js} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename test/projects/broken/{invalid-svglint-config.js => broken-svglint-config.js} (100%) diff --git a/test/cli.spec.js b/test/cli.spec.js index f809b70..a8f7151 100644 --- a/test/cli.spec.js +++ b/test/cli.spec.js @@ -62,9 +62,9 @@ describe("Configuration files", function() { expect(exitCode).toBe(3); }); - it("should fail with an invalid configuration file", async function(){ + it("should fail with an broken configuration file", async function(){ const { failed, exitCode } = await execCliWith([ - "--config", "./test/projects/broken/invalid-svglint-config.js" + "--config", "./test/projects/broken/broken-svglint-config.js" ]); expect(failed).toBeTruthy(); expect(exitCode).toBe(4); diff --git a/test/projects/broken/invalid-svglint-config.js b/test/projects/broken/broken-svglint-config.js similarity index 100% rename from test/projects/broken/invalid-svglint-config.js rename to test/projects/broken/broken-svglint-config.js From 75f7ac7b9b28a59cfc896582e33f549ae729852a Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Mon, 24 Oct 2022 22:19:47 +0200 Subject: [PATCH 7/8] refactor: update exit codes - Combine exit codes related to configuration into one - Add exit code for SIGINT - Define `EXIT_CODES` object for readability and to avoid duplication --- bin/cli.js | 23 +++++++++++++++++++---- test/cli.spec.js | 2 +- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index 2512b05..9bafdf2 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -17,6 +17,14 @@ const GUI = new gui(); const logger = Logger(""); +const EXIT_CODES = Object.freeze({ + success: 0, + violations: 1, + unexpected: 2, + interrupted: 3, + configuration: 4, +}); + // used by meow's loud reject // eslint-disable-next-line no-console console.error = logger.error.bind(logger); @@ -24,7 +32,12 @@ console.error = logger.error.bind(logger); // Pretty logs all errors, then exits process.on("uncaughtException", err => { logger.error(err); - process.exit(2); + process.exit(EXIT_CODES.unexpected); +}); + +// Handle SIGINT +process.on("SIGINT", () => { + process.exit(EXIT_CODES.interrupted); }); // Generates the CLI binding using meow @@ -69,12 +82,12 @@ process.on("exit", () => { logger.debug("No configuration file found"); if (cli.flags.config) { logger.error("Configuration file not found"); - process.exit(3); + process.exit(EXIT_CODES.configuration); } } } catch (e) { logger.error(`Failed to parse config: ${e.stack}`); - process.exit(4); + process.exit(EXIT_CODES.configuration); } // lint all the files @@ -85,7 +98,9 @@ process.on("exit", () => { --activeLintings; logger.debug("Linting done,", activeLintings, "to go"); if (activeLintings <= 0) { - process.exit(hasErrors ? 1 : 0); + process.exit( + hasErrors ? EXIT_CODES.violations : EXIT_CODES.success + ); } }; files.forEach(filePath => { diff --git a/test/cli.spec.js b/test/cli.spec.js index a8f7151..ef058bd 100644 --- a/test/cli.spec.js +++ b/test/cli.spec.js @@ -59,7 +59,7 @@ describe("Configuration files", function() { "--config", "./this/file/does/not-exist.js" ]); expect(failed).toBeTruthy(); - expect(exitCode).toBe(3); + expect(exitCode).toBe(4); }); it("should fail with an broken configuration file", async function(){ From 96fa6e33beebcbbc814e947ceb106c46b42a5c3f Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Mon, 24 Oct 2022 22:21:22 +0200 Subject: [PATCH 8/8] chore: update formatting of cli.spec.js --- test/cli.spec.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/cli.spec.js b/test/cli.spec.js index ef058bd..ac1f72c 100644 --- a/test/cli.spec.js +++ b/test/cli.spec.js @@ -55,17 +55,17 @@ describe("CLI", function(){ describe("Configuration files", function() { it("should fail with an non-existent configuration file", async function(){ - const { failed, exitCode } = await execCliWith([ - "--config", "./this/file/does/not-exist.js" - ]); + const { failed, exitCode } = await execCliWith( + ["--config", "./this/file/does/not-exist.js"] + ); expect(failed).toBeTruthy(); expect(exitCode).toBe(4); }); - it("should fail with an broken configuration file", async function(){ - const { failed, exitCode } = await execCliWith([ - "--config", "./test/projects/broken/broken-svglint-config.js" - ]); + it("should fail with a broken configuration file", async function(){ + const { failed, exitCode } = await execCliWith( + ["--config", "./test/projects/broken/broken-svglint-config.js"] + ); expect(failed).toBeTruthy(); expect(exitCode).toBe(4); });