Merge pull request #5 from hidakatsuya/always-show-the-report

Always report lint results to Job Summaries

Author: hidakatsuya

.github/workflows/test.yml

@@ -12,7 +12,7 @@ jobs:
     - run: npm test
 
   # test action works running from the graph
-  test:
+  action-test-success:
     runs-on: ubuntu-latest
 
     env:
@@ -23,13 +23,23 @@ jobs:
     - uses: ./
       with:
         result-path: "__tests__/xml/success*.xml"
-        ignore-warning: false
+
+  action-test-success-with-warnings:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v3
     - uses: ./
       with:
         result-path: "__tests__/xml/failure2.xml"
-        ignore-warning: true
+        fail-on-warning: false
+
+  action-test-error:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v3
     - uses: ./
-      if: env.RUN_FAILURE_TEST == 'true'
       with:
         result-path: "__tests__/xml/failure*.xml"
-        ignore-warning: false
+      continue-on-error: true

README.md

@@ -25,9 +25,9 @@ Alternatively, you can specify a major version, such as `hidakatsuya/action-repo
 Indicates the relative path from the working directory to Android Lint result XML file.
 Path patterns by [@actions/glob](https://www.npmjs.com/package/@actions/glob) can also be specified.
 
-### ignore-warning (optional)
+### fail-on-warning (optional)
 
-Indicates whether warnings should be ignored as build failures. Default is `false`.
+Indicates whether the action should fail if there is a severity warning issue. Default is `true`.
 
 ### follow-symbolic-links (optional)
 

__tests__/check.test.js

@@ -2,10 +2,10 @@ const { check } = require("../src/check")
 const path = require("path")
 
 describe("single XML file", () => {
-  test("when the result is success", async () => {
+  test("when the results is success", async () => {
     const results = await check({ pathPattern: path.join(__dirname, "xml/success1.xml") })
 
-    expect(results.isPassed).toBe(true)
+    expect(results.status).toEqual("success")
     expect(results.failures.length).toEqual(0)
     expect(results.results.length).toEqual(1)
 
@@ -16,15 +16,16 @@ describe("single XML file", () => {
     expect(result.warnings.length).toEqual(0)
   })
 
-  test("when the result is failure", async () => {
+  test("when the results is error including warning", async () => {
     const results = await check({ pathPattern: path.join(__dirname, "xml/failure1.xml") })
 
-    expect(results.isPassed).toBe(false)
+    expect(results.status).toEqual("error")
     expect(results.failures.length).toEqual(1)
     expect(results.results.length).toEqual(1)
 
     const { result } = results.results[0]
 
+    expect(result.status).toEqual("error")
     expect(result.issues.length).toEqual(2)
 
     expect(result.warnings.length).toEqual(1)
@@ -33,21 +34,39 @@ describe("single XML file", () => {
     expect(result.errors.length).toEqual(1)
     expect(result.errors[0].message).toMatch("The resource `R.color.purple_500` appears to be unused")
   })
+
+  test("when the results is warning only", async () => {
+    const results = await check({ pathPattern: path.join(__dirname, "xml/failure2.xml") })
+
+    expect(results.status).toEqual("warning")
+    expect(results.failures.length).toEqual(1)
+    expect(results.results.length).toEqual(1)
+
+    const { result } = results.results[0]
+
+    expect(result.status).toEqual("warning")
+    expect(result.issues.length).toEqual(1)
+
+    expect(result.warnings.length).toEqual(1)
+    expect(result.warnings[0].message).toMatch("The resource `R.color.purple_700` appears to be unused")
+
+    expect(result.errors.length).toEqual(0)
+  })
 })
 
 describe("multiple XML files", () => {
   test("when the all results are success", async () => {
     const results = await check({ pathPattern: path.join(__dirname, "xml/success*.xml") })
 
-    expect(results.isPassed).toBe(true)
+    expect(results.status).toEqual("success")
     expect(results.failures.length).toEqual(0)
     expect(results.results.length).toEqual(2)
   })
 
   test("when the all results are failure", async () => {
     const results = await check({ pathPattern: path.join(__dirname, "xml/failure*.xml") })
 
-    expect(results.isPassed).toBe(false)
+    expect(results.status).toEqual("error")
     expect(results.failures.length).toEqual(2)
     expect(results.results.length).toEqual(2)
   })
@@ -59,30 +78,12 @@ describe("multiple XML files", () => {
     ]
     const results = await check({ pathPattern })
 
-    expect(results.isPassed).toBe(false)
+    expect(results.status).toEqual("error")
     expect(results.failures.length).toEqual(1)
     expect(results.results.length).toEqual(2)
   })
 })
 
-describe("ignoreWarning", () => {
-  test("when true", async () => {
-    const results = await check({ pathPattern: path.join(__dirname, "xml/failure2.xml"), ignoreWarning: true })
-
-    expect(results.isPassed).toBe(true)
-    expect(results.failures.length).toEqual(0)
-    expect(results.results.length).toEqual(1)
-  })
-
-  test("when false", async () => {
-    const results = await check({ pathPattern: path.join(__dirname, "xml/failure2.xml"), ignoreWarning: false })
-
-    expect(results.isPassed).toBe(false)
-    expect(results.failures.length).toEqual(1)
-    expect(results.results.length).toEqual(1)
-  })
-})
-
 test("when the XML file can't be found", async () => {
   await expect(check({ pathPattern: path.join(__dirname, "xml/unknown.xml") }))
     .rejects.toThrow("No XML file found")
@@ -97,7 +98,7 @@ describe("basic glob path pattern", () => {
   test("**/xml/failure*.xml", async () => {
     const results = await check({ pathPattern: "**/xml/failure*.xml" })
 
-    expect(results.isPassed).toBe(false)
+    expect(results.status).toEqual("error")
     expect(results.failures.length).toEqual(2)
     expect(results.results.length).toEqual(2)
   })

__tests__/report.test.js

@@ -4,55 +4,116 @@ const { Issue, Result, Results } = require("../src/check")
 
 const baseDir = "/path/to"
 
-function buildIssue(no = 1) {
+function buildIssue({ severity, identifier = 1 }) {
   return new Issue({
     location: {
-      "@_file": `/path/to/app/src/source${no}.kt`,
-      "@_line": `${no}`,
+      "@_file": `/path/to/app/src/source${identifier}.kt`,
+      "@_line": `${identifier}`,
     },
     "@_id": "SomeError",
-    "@_severity": "Warning",
-    "@_message": `Error message${no}`,
+    "@_severity": severity,
+    "@_message": `Error message${identifier}`,
     "@_summary": "Some Error",
     "@_errorLine1": "line1",
     "@_errorLine2": "line2"
   })
 }
 
 beforeEach(() => {
+  core.summary.emptyBuffer()
   core.summary.write = jest.fn()
 })
 
-test("single result", async () => {
-  const result = new Result([buildIssue()])
-  const results = new Results([{ path: "/path/to/app/build/result.xml", result }])
+afterEach(() => {
+  core.summary.write.mockReset()
+})
 
-  await report({ results, core, baseDir })
+describe("single result", () => {
+  test("when the results is success", async () => {
+    const result = new Result([])
+    const results = new Results([{ path: "/path/to/app/build/result.xml", result }])
 
-  expect(core.summary.write.mock.calls.length).toEqual(1)
+    await report({ results, core, baseDir })
 
-  const summary = core.summary.stringify()
+    expect(core.summary.write.mock.calls.length).toEqual(1)
+
+    const summary = core.summary.stringify()
+
+    expect(summary).toMatch([
+      "<h2>✔ Android Lint</h2>",
+      "<p>No issue.</p>"
+    ].join("\n"))
+
+    expect(summary).not.toMatch("<h3>app/build/result.xml</h3>")
+  })
+
+  test("when the results is error", async () => {
+    const result = new Result([buildIssue({ severity: "Error", identifier: 1 })])
+    const results = new Results([{ path: "/path/to/app/build/result.xml", result }])
+
+    await report({ results, core, baseDir })
+
+    expect(core.summary.write.mock.calls.length).toEqual(1)
 
-  expect(summary).toMatch("<h2>Android Lint</h2>")
-  expect(summary).toMatch("<h3>app/build/result.xml</h3>")
-  expect(summary).toMatch("<details><summary>⚠️ Warnings</summary>")
-  expect(summary).toMatch("#### app/src/source1.kt (1 issues)")
-  expect(summary).toMatch("* **Line#1** - SomeError: Error message1")
-  expect(summary).toMatch("  ```")
-  expect(summary).toMatch("  line1")
-  expect(summary).toMatch("  line2")
-  expect(summary).toMatch("  ```")
+    const summary = core.summary.stringify()
+
+    expect(summary).toMatch([
+      "<h2>❌ Android Lint</h2>",
+      "<h3>app/build/result.xml</h3>",
+      "<details><summary>❌ Errors</summary>",
+      "",
+      "#### app/src/source1.kt (1 issues)",
+      "* **Line#1** - SomeError: Error message1",
+      "  ```",
+      "  line1",
+      "  line2",
+      "  ```"
+    ].join("\n"))
+
+    expect(summary).not.toMatch("No issue")
+    expect(summary).not.toMatch("Warnings")
+  })
+
+  test("when the results is warning", async () => {
+    const result = new Result([buildIssue({ severity: "Warning", identifier: 2 })])
+    const results = new Results([{ path: "/path/to/app/build/result.xml", result }])
+
+    await report({ results, core, baseDir })
+
+    expect(core.summary.write.mock.calls.length).toEqual(1)
+
+    const summary = core.summary.stringify()
+
+    expect(summary).toMatch([
+      "<h2>⚠ Android Lint</h2>",
+      "<h3>app/build/result.xml</h3>",
+      "<details><summary>⚠️ Warnings</summary>",
+      "",
+      "#### app/src/source2.kt (1 issues)",
+      "* **Line#2** - SomeError: Error message2",
+      "  ```",
+      "  line1",
+      "  line2",
+      "  ```"
+    ].join("\n"))
+
+    expect(summary).not.toMatch("No issue")
+    expect(summary).not.toMatch("Errors")
+  })
 })
 
 test("multiple results", async () => {
   const results = new Results([
     {
       path: "/path/to/app/build/result1.xml",
-      result: new Result([buildIssue(1), buildIssue(2)])
+      result: new Result([
+        buildIssue({ identifier: 1, severity: "Error" }),
+        buildIssue({ identifier: 2, severity: "Warning" })
+      ])
     },
     {
       path: "/path/to/app/build/result2.xml",
-      result: new Result([buildIssue(3)])
+      result: new Result([buildIssue({ identifier: 3, severity: "Error" })])
     }
   ])
 
@@ -62,12 +123,36 @@ test("multiple results", async () => {
 
   const summary = core.summary.stringify()
 
-  expect(summary).toMatch("<h2>Android Lint</h2>")
-  expect(summary).toMatch("<h3>app/build/result1.xml</h3>")
-  expect(summary).toMatch("<details><summary>⚠️ Warnings</summary>")
-  expect(summary).toMatch("#### app/src/source1.kt (1 issues)")
-  expect(summary).toMatch("* **Line#1** - SomeError: Error message1")
-  expect(summary).toMatch("* **Line#2** - SomeError: Error message2")
-  expect(summary).toMatch("#### app/src/source2.kt (1 issues)")
-  expect(summary).toMatch("* **Line#3** - SomeError: Error message3")
+  expect(summary).toMatch([
+    "<h2>❌ Android Lint</h2>",
+    "<h3>app/build/result1.xml</h3>",
+    "<details><summary>❌ Errors</summary>",
+    "",
+    "#### app/src/source1.kt (1 issues)",
+    "* **Line#1** - SomeError: Error message1",
+    "  ```",
+    "  line1",
+    "  line2",
+    "  ```",
+    "</details>",
+    "<details><summary>⚠️ Warnings</summary>",
+    "",
+    "#### app/src/source2.kt (1 issues)",
+    "* **Line#2** - SomeError: Error message2",
+    "  ```",
+    "  line1",
+    "  line2",
+    "  ```",
+    "</details>",
+    "<h3>app/build/result2.xml</h3>",
+    "<details><summary>❌ Errors</summary>",
+    "",
+    "#### app/src/source3.kt (1 issues)",
+    "* **Line#3** - SomeError: Error message3",
+    "  ```",
+    "  line1",
+    "  line2",
+    "  ```",
+    "</details>"
+  ].join("\n"))
 })