refactor process results

a-b-r-o-w-n · Jun 2, 2024 · cc31b55 · cc31b55
1 parent a82d66e
commit cc31b55
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 72 deletions.
diff --git a/__tests__/processResults.test.ts b/__tests__/processResults.test.ts
@@ -1,5 +1,7 @@
+import { error, warning, startGroup, endGroup } from "@actions/core";
+
 import inputs from "../src/inputs";
-import { CHECK_NAME, processResults } from "../src/processResults";
+import { processResults } from "../src/processResults";
 
 jest.mock("@actions/core", () => ({
   debug: jest.fn(),
@@ -16,71 +18,130 @@ jest.mock("../src/inputs", () => ({
 
 const errorMsg = {
   line: 10,
+  endLine: 10,
+  column: 1,
+  endColumn: 10,
   severity: 2,
   ruleId: "no-var",
   message: "no var allowed",
 };
 
 const warnMsg = {
   line: 5,
+  endLine: 5,
+  column: 1,
+  endColumn: 10,
   severity: 1,
   ruleId: "no-console",
   message: "no console allowed",
 };
 
+const multilineMsg = {
+  line: 5,
+  endLine: 10,
+  column: 1,
+  endColumn: 10,
+  severity: 2,
+  ruleId: "no-multiline",
+  message: "no multiline allowed",
+};
+
 const invalidMsg = {
   line: 1,
   severity: 2,
+  column: 1,
+  endColumn: 10,
   ruleId: undefined,
   message: "bad rule",
 };
 
 const mockResults = [
   {
     filePath: "/foo",
-    messages: [errorMsg, warnMsg, { ...errorMsg, line: 20 }],
+    messages: [errorMsg, warnMsg, { ...errorMsg, line: 20, endLine: 20 }],
   },
   {
     filePath: "/bar",
-    messages: [errorMsg, invalidMsg],
+    messages: [errorMsg, invalidMsg, multilineMsg],
   },
 ];
 
-it("returns check endpoint payload with correct annotations", () => {
+it("returns correct error count and warning count", () => {
   // @ts-expect-error
-  const payload = processResults(mockResults);
-
-  expect(payload).toMatchObject({
-    conclusion: "failure",
-    output: {
-      title: CHECK_NAME,
-      summary: "3 error(s) found",
-      annotations: expect.any(Array),
-    },
-  });
+  const result = processResults(mockResults);
 
-  const annotations = payload.output?.annotations;
-  expect(annotations).toHaveLength(4);
+  expect(result).toMatchObject({
+    errorCount: 4,
+    warningCount: 1,
+  });
 });
 
-it("passes the check if there are no errors", () => {
-  const payload = processResults([
-    {
-      filePath: "/bar",
-      // @ts-expect-error
-      messages: [warnMsg, invalidMsg],
-    },
-  ]);
+it("logs each lint result with file annotations", () => {
+  // @ts-expect-error
+  processResults(mockResults);
+
+  expect(error).toHaveBeenCalledTimes(4);
+  expect(warning).toHaveBeenCalledTimes(1);
+
+  expect(error).toHaveBeenNthCalledWith(1, "[no-var] no var allowed", {
+    file: "/foo",
+    startLine: 10,
+    endLine: 10,
+    startColumn: 1,
+    endColumn: 10,
+  });
+
+  expect(warning).toHaveBeenNthCalledWith(1, "[no-console] no console allowed", {
+    file: "/foo",
+    startLine: 5,
+    endLine: 5,
+    startColumn: 1,
+    endColumn: 10,
+  });
+
+  expect(error).toHaveBeenNthCalledWith(2, "[no-var] no var allowed", {
+    file: "/foo",
+    startLine: 20,
+    endLine: 20,
+    startColumn: 1,
+    endColumn: 10,
+  });
 
-  expect(payload.conclusion).toEqual("success");
+  expect(error).toHaveBeenNthCalledWith(3, "[no-var] no var allowed", {
+    file: "/bar",
+    startLine: 10,
+    endLine: 10,
+    startColumn: 1,
+    endColumn: 10,
+  });
+
+  expect(error).toHaveBeenNthCalledWith(4, "[no-multiline] no multiline allowed", {
+    file: "/bar",
+    startLine: 5,
+    endLine: 10,
+  });
 });
 
 it("does not include warnings if quiet option is true", () => {
   Object.defineProperty(inputs, "quiet", {
     get: () => true,
   });
   // @ts-expect-error
-  const payload = processResults(mockResults);
-  expect(payload.output?.annotations).toHaveLength(3);
-  expect(payload.output?.annotations?.some((a) => a.annotation_level === "warning")).toBe(false);
+  const result = processResults(mockResults);
+  expect(result).toMatchObject({
+    errorCount: 4,
+    warningCount: 0,
+  });
+  expect(warning).not.toHaveBeenCalledWith("[no-console] no console allowed", expect.any(Object));
+});
+
+it("groups log output by file", () => {
+  // @ts-expect-error
+  processResults(mockResults);
+
+  expect(startGroup).toHaveBeenCalledTimes(2);
+  expect(endGroup).toHaveBeenCalledTimes(2);
+
+  expect(startGroup).toHaveBeenNthCalledWith(1, "/foo");
+  expect(startGroup).toHaveBeenNthCalledWith(2, "/bar");
 });
diff --git a/jest.config.js b/jest.config.js
@@ -2,4 +2,5 @@ module.exports = {
   preset: "ts-jest",
   testPathIgnorePatterns: ["/node_modules/", "<rootDir>/__tests__/fixtures"],
   collectCoverageFrom: ["src/**/*.ts"],
+  resetMocks: true,
 };
diff --git a/lib/index.js b/lib/index.js
@@ -173743,27 +173743,13 @@ const tslib_1 = __nccwpck_require__(4351);
 const core = tslib_1.__importStar(__nccwpck_require__(42186));
 const inputs_1 = tslib_1.__importDefault(__nccwpck_require__(7063));
 const { GITHUB_WORKSPACE } = process.env;
-function logFileAnnotations(filePath, annotations) {
-    core.startGroup(filePath);
-    annotations.forEach((a) => {
-        const { message, severity } = a, annotationDetails = tslib_1.__rest(a, ["message", "severity"]);
-        core.debug(JSON.stringify(a));
-        if (severity === "warning") {
-            core.warning(message, annotationDetails);
-        }
-        else {
-            core.warning(message, annotationDetails);
-        }
-    });
-    core.endGroup();
-}
 function processResults(results) {
     let errorCount = 0;
     let warningCount = 0;
     for (const result of results) {
         const { filePath, messages } = result;
         const relFilePath = filePath.replace(`${GITHUB_WORKSPACE}/`, "");
-        const fileAnnotations = [];
+        core.startGroup(relFilePath);
         for (const lintMessage of messages) {
             const { line, endLine, severity, ruleId, message, column, endColumn } = lintMessage;
             // if ruleId is null, it's likely a parsing error, so let's skip it
@@ -173781,18 +173767,23 @@ function processResults(results) {
                 warningCount++;
             }
             const isMultiLine = endLine && endLine !== line;
-            fileAnnotations.push({
+            const annotationDetails = {
                 file: relFilePath,
                 startLine: line,
                 endLine,
                 // only add column info if error is on a single line
                 startColumn: isMultiLine ? undefined : column,
                 endColumn: isMultiLine ? undefined : endColumn,
-                severity: severity === 2 ? "failure" : "warning",
-                message: `[${ruleId}] ${message}`,
-            });
+            };
+            core.debug(JSON.stringify(Object.assign({ message: `[${ruleId}] ${message}` }, annotationDetails)));
+            if (severity === 1) {
+                core.warning(`[${ruleId}] ${message}`, annotationDetails);
+            }
+            else {
+                core.error(`[${ruleId}] ${message}`, annotationDetails);
+            }
         }
-        logFileAnnotations(relFilePath, fileAnnotations);
+        core.endGroup();
     }
     return {
         errorCount,

diff --git a/src/processResults.ts b/src/processResults.ts
@@ -10,39 +10,23 @@ type LintResult = {
   warningCount: number;
 };
 
-type Annotation = {
+type AnnotationDetails = {
   file: string;
-  message: string;
-  severity: "warning" | "failure";
   startColumn?: number;
   endColumn?: number;
   startLine?: number;
   endLine?: number;
 };
 
-function logFileAnnotations(filePath: string, annotations: Annotation[]) {
-  core.startGroup(filePath);
-  annotations.forEach((a) => {
-    const { message, severity, ...annotationDetails } = a;
-
-    core.debug(JSON.stringify(a));
-    if (severity === "warning") {
-      core.warning(message, annotationDetails);
-    } else {
-      core.warning(message, annotationDetails);
-    }
-  });
-  core.endGroup();
-}
-
 export function processResults(results: ESLint.LintResult[]): LintResult {
   let errorCount = 0;
   let warningCount = 0;
 
   for (const result of results) {
     const { filePath, messages } = result;
     const relFilePath = filePath.replace(`${GITHUB_WORKSPACE}/`, "");
-    const fileAnnotations: Annotation[] = [];
+
+    core.startGroup(relFilePath);
 
     for (const lintMessage of messages) {
       const { line, endLine, severity, ruleId, message, column, endColumn } = lintMessage;
@@ -63,19 +47,24 @@ export function processResults(results: ESLint.LintResult[]): LintResult {
 
       const isMultiLine = endLine && endLine !== line;
 
-      fileAnnotations.push({
+      const annotationDetails: AnnotationDetails = {
         file: relFilePath,
         startLine: line,
         endLine,
         // only add column info if error is on a single line
         startColumn: isMultiLine ? undefined : column,
         endColumn: isMultiLine ? undefined : endColumn,
-        severity: severity === 2 ? "failure" : "warning",
-        message: `[${ruleId}] ${message}`,
-      });
+      };
+
+      core.debug(JSON.stringify({ message: `[${ruleId}] ${message}`, ...annotationDetails }));
+      if (severity === 1) {
+        core.warning(`[${ruleId}] ${message}`, annotationDetails);
+      } else {
+        core.error(`[${ruleId}] ${message}`, annotationDetails);
+      }
     }
 
-    logFileAnnotations(relFilePath, fileAnnotations);
+    core.endGroup();
   }
 
   return {