fix: changed error detection from stderr to command return code to pr…

…event yarn warnings from crashing license-auditor process
brainhubeu · Dec 30, 2024 · 7c95390 · 7c95390
1 parent 368b25f
commit 7c95390
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 30 deletions.
diff --git a/packages/core/src/dependency-finder/exec-command.ts b/packages/core/src/dependency-finder/exec-command.ts
@@ -2,14 +2,19 @@ import { exec } from "node:child_process";
 
 import { ExecCommandException } from "../exceptions/index.js";
 
+type ExecCommandResult = {
+  stdout: string;
+  stderr: string;
+};
+
 export async function execCommand(
   command: string,
   cwd: string,
   verbose?: boolean | undefined,
-): Promise<string> {
+): Promise<ExecCommandResult> {
   return new Promise((resolve, reject) => {
     exec(command, { cwd }, (error, stdout, stderr) => {
-      if (error || (stderr && !stderr.includes("Debugger attached"))) {
+      if (error) {
         reject(
           new ExecCommandException(
             error?.stack && verbose
@@ -24,7 +29,7 @@ export async function execCommand(
         );
       }
 
-      resolve(stdout);
+      resolve({ stdout, stderr });
     });
   });
 }
diff --git a/packages/core/src/dependency-finder/pnpm.ts b/packages/core/src/dependency-finder/pnpm.ts
@@ -10,7 +10,7 @@ export async function findPnpmDependencies(
   production?: boolean | undefined,
   verbose?: boolean | undefined,
 ): Promise<DependenciesResult> {
-  const output = await execCommand(
+  const { stdout: output } = await execCommand(
     production ? findPnpmProdDepsCommand : findPnpmDepsCommand,
     projectRoot,
     verbose,

diff --git a/packages/core/src/dependency-finder/yarn-classic.ts b/packages/core/src/dependency-finder/yarn-classic.ts
@@ -31,7 +31,7 @@ export async function findYarnClassicDependencies(
   production?: boolean | undefined,
   verbose?: boolean | undefined,
 ): Promise<DependenciesResult> {
-  const output = await execCommand(
+  const { stdout: output } = await execCommand(
     production ? findYarnClassicProdDepsCommand : findYarnClassicDepsCommand,
     projectRoot,
     verbose,

diff --git a/test/fixtures.ts b/test/fixtures.ts
@@ -128,6 +128,23 @@ export const yarnFixture = test.extend<TestContext>({
   },
 });
 
+export const yarnWithInvalidGithubDepFixture = test.extend<TestContext>({
+  // biome-ignore lint/correctness/noEmptyPattern: destructuring pattern is required in fixture
+  testDirectory: async ({}, use) => {
+    const testDirectory = path.resolve(
+      TEST_TEMP_DIRECTORY,
+      `testProject-${Math.random().toString(36).substring(2)}`,
+    );
+    await fs.cp(path.resolve(TEST_PROJECTS_DIRECTORY, "invalidGithubDepYarn"), testDirectory, {
+      recursive: true,
+    });
+
+    await use(testDirectory);
+
+    await fs.rm(testDirectory, { recursive: true });
+  },
+});
+
 export const monorepoFixture = test.extend<TestContext>({
   // biome-ignore lint/correctness/noEmptyPattern: destructuring pattern is required in fixture
   testDirectory: async ({}, use) => {

diff --git a/test/test/yarn.test.ts b/test/test/yarn.test.ts
@@ -2,7 +2,7 @@ import * as fs from "node:fs/promises";
 import * as path from "node:path";
 import type { JsonResults } from "@license-auditor/data";
 import { describe, expect } from "vitest";
-import { yarnFixture } from "../fixtures";
+import { yarnFixture, yarnWithInvalidGithubDepFixture } from "../fixtures";
 import { addToPackageJson } from "../utils/add-to-package-json";
 import { getCliPath } from "../utils/get-cli-path";
 import { readJsonFile } from "../utils/read-json-file";
@@ -24,13 +24,13 @@ describe("yarn", () => {
           cwd: testDirectory,
         });
 
+        expect(errorCode).toBe(0);
+        expect(output).toContain("160 licenses are compliant");
+
         const jsonOutput: JsonResults = await readJsonFile(
           path.join(testDirectory, "license-auditor.results.json"),
         );
 
-        expect(errorCode).toBe(0);
-        expect(output).toContain("160 licenses are compliant");
-
         const addedPackage = jsonOutput.whitelist.find(
           (result) => result.packageName === "[email protected]",
         );
@@ -53,13 +53,13 @@ describe("yarn", () => {
           cwd: testDirectory,
         });
 
+        expect(errorCode).toBe(0);
+        expect(output).toContain("160 licenses are compliant");
+
         const jsonOutput: JsonResults = await readJsonFile(
           path.join(testDirectory, "license-auditor.results.json"),
         );
 
-        expect(errorCode).toBe(0);
-        expect(output).toContain("160 licenses are compliant");
-
         const addedPackage = jsonOutput.whitelist.find(
           (result) => result.packageName === "[email protected]",
         );
@@ -82,13 +82,13 @@ describe("yarn", () => {
           cwd: testDirectory,
         });
 
+        expect(errorCode).toBe(0);
+        expect(output).toContain("160 licenses are compliant");
+
         const jsonOutput: JsonResults = await readJsonFile(
           path.join(testDirectory, "license-auditor.results.json"),
         );
 
-        expect(errorCode).toBe(0);
-        expect(output).toContain("160 licenses are compliant");
-
         const addedPackage = jsonOutput.whitelist.find(
           (result) => result.packageName === "[email protected]",
         );
@@ -111,13 +111,13 @@ describe("yarn", () => {
           cwd: testDirectory,
         });
 
+        expect(errorCode).toBe(0);
+        expect(output).toContain("160 licenses are compliant");
+
         const jsonOutput: JsonResults = await readJsonFile(
           path.join(testDirectory, "license-auditor.results.json"),
         );
 
-        expect(errorCode).toBe(0);
-        expect(output).toContain("160 licenses are compliant");
-
         const addedPackage = jsonOutput.whitelist.find(
           (result) => result.packageName === "[email protected]",
         );
@@ -147,13 +147,13 @@ describe("yarn", () => {
           cwd: testDirectory,
         });
 
+        expect(errorCode).toBe(0);
+        expect(output).toContain("160 licenses are compliant");
+
         const jsonOutput: JsonResults = await readJsonFile(
           path.join(testDirectory, "license-auditor.results.json"),
         );
 
-        expect(errorCode).toBe(0);
-        expect(output).toContain("160 licenses are compliant");
-
         const addedPackage = jsonOutput.whitelist.find(
           (result) => result.packageName === "[email protected]",
         );
@@ -189,13 +189,13 @@ describe("yarn", () => {
           cwd: testDirectory,
         });
 
+        expect(errorCode).toBe(0);
+        expect(output).toContain("160 licenses are compliant");
+
         const jsonOutput: JsonResults = await readJsonFile(
           path.join(testDirectory, "license-auditor.results.json"),
         );
 
-        expect(errorCode).toBe(0);
-        expect(output).toContain("160 licenses are compliant");
-
         const addedPackage = jsonOutput.whitelist.find(
           (result) => result.packageName === "[email protected]",
         );
@@ -220,13 +220,13 @@ describe("yarn", () => {
           cwd: testDirectory,
         });
 
+        expect(errorCode).toBe(0);
+        expect(output).toContain("67 licenses are compliant");
+
         const jsonOutput: JsonResults = await readJsonFile(
           path.join(testDirectory, "license-auditor.results.json"),
         );
 
-        expect(errorCode).toBe(0);
-        expect(output).toContain("67 licenses are compliant");
-
         const addedPackage = jsonOutput.whitelist.find(
           (result) => result.packageName === "[email protected]",
         );
@@ -253,13 +253,13 @@ describe("yarn", () => {
         cwd: testDirectory,
       });
 
+      expect(errorCode).toBe(0);
+      expect(output).toContain("66 licenses are compliant");
+
       const jsonOutput: JsonResults = await readJsonFile(
         path.join(testDirectory, "license-auditor.results.json"),
       );
 
-      expect(errorCode).toBe(0);
-      expect(output).toContain("66 licenses are compliant");
-
       const addedPackage = jsonOutput.whitelist.find(
         (result) => result.packageName === "[email protected]",
       );
@@ -292,11 +292,24 @@ describe("yarn", () => {
           cwd: testDirectory,
         });
 
+        expect(errorCode).toBe(1);
         expect(output).toContain("Invalid configuration file at");
         expect(output).toContain("Expected array, received string");
         expect(output).toContain("Expected array, received number");
         expect(output).toContain("Expected object, received string");
       },
     );
   });
+  describe('github dependency with invalid url', () => {
+    yarnWithInvalidGithubDepFixture('github dependency with invalid url', async ({ testDirectory }) => {
+      const { output, errorCode } = await runCliCommand({
+        command: "npx",
+        args: [getCliPath()],
+        cwd: testDirectory,
+      });
+
+      expect(errorCode).toBe(0);
+      expect(output).toContain("1 license is compliant");
+    });
+  });
 });
diff --git a/test/testProjects/invalidGithubDepYarn/package.json b/test/testProjects/invalidGithubDepYarn/package.json
@@ -0,0 +1,13 @@
+{
+  "name": "testproject",
+  "version": "1.0.0",
+  "main": "index.js",
+  "packageManager": "[email protected]",
+  "scripts": {
+    "run": "echo \"Nope\""
+  },
+  "private": true,
+  "dependencies": {
+    "swich": "git+ssh://[email protected]:Lukasz-pluszczewski/swich.git"
+  }
+}
diff --git a/test/testProjects/invalidGithubDepYarn/yarn.lock b/test/testProjects/invalidGithubDepYarn/yarn.lock
@@ -0,0 +1,7 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+"swich@git+ssh://[email protected]:Lukasz-pluszczewski/swich.git":
+  version "1.2.0"
+  resolved "git+ssh://[email protected]:Lukasz-pluszczewski/swich.git#027b2f0118b15c001d369b56d4f893072c27d901"