feat(node): Add logic for combining single sentence issues lines

When not combining lines, trim them to normalize output from different NPM versions which sometimes has trailing whitespaces. Resolves #7071. Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Jun 9, 2024 · 811daef · 811daef
1 parent 1b5a925
commit 811daef
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 4 deletions.
diff --git a/...-managers/node/src/funTest/assets/projects/synthetic/npm-version-urls-expected-output.yml b/...-managers/node/src/funTest/assets/projects/synthetic/npm-version-urls-expected-output.yml
@@ -5193,13 +5193,13 @@ issues:
   severity: "HINT"
 - timestamp: "1970-01-01T00:00:00Z"
   source: "NPM"
-  message: "skipping integrity check for git dependency ssh://[email protected]/jonschlinkert/gulp-format-md.git "
+  message: "skipping integrity check for git dependency ssh://[email protected]/jonschlinkert/gulp-format-md.git"
   severity: "HINT"
 - timestamp: "1970-01-01T00:00:00Z"
   source: "NPM"
-  message: "skipping integrity check for git dependency ssh://[email protected]/mochajs/mocha.git "
+  message: "skipping integrity check for git dependency ssh://[email protected]/mochajs/mocha.git"
   severity: "HINT"
 - timestamp: "1970-01-01T00:00:00Z"
   source: "NPM"
-  message: "skipping integrity check for git dependency ssh://[email protected]/tinydesk/angular-tileview.git "
+  message: "skipping integrity check for git dependency ssh://[email protected]/tinydesk/angular-tileview.git"
   severity: "HINT"
diff --git a/plugins/package-managers/node/src/main/kotlin/Npm.kt b/plugins/package-managers/node/src/main/kotlin/Npm.kt
@@ -693,8 +693,19 @@ internal fun List<String>.groupLines(vararg markers: String): List<String> {
         collapsedLines[collapsedLines.size - 1] = collapsedLines.last().removePrefix(previousPrefix).trimStart()
     }
 
-    return collapsedLines.takeWhile {
+    val nonFooterLines = collapsedLines.takeWhile {
         // Skip any footer as a whole.
         it != "A complete log of this run can be found in:"
     }
+
+    // If no lines but the last end with a dot, assume the message to be a single sentence.
+    return if (
+        nonFooterLines.size > 1 &&
+        nonFooterLines.last().endsWith('.') &&
+        nonFooterLines.subList(0, nonFooterLines.size - 1).none { it.endsWith('.') }
+    ) {
+        listOf(nonFooterLines.joinToString(" "))
+    } else {
+        nonFooterLines.map { it.trim() }
+    }
 }
diff --git a/plugins/package-managers/node/src/test/kotlin/NpmTest.kt b/plugins/package-managers/node/src/test/kotlin/NpmTest.kt
@@ -115,5 +115,25 @@ class NpmTest : WordSpec({
                 "deprecated [email protected]: CoffeeScript on NPM has moved to \"coffeescript\" (no hyphen)"
             )
         }
+
+        "treat a single block of errors as one issue" {
+            val output = """
+                npm ERR! code EEXIST
+                npm ERR! syscall mkdir
+                npm ERR! path G:\Git\lsp-sample\node_modules.staging
+                npm ERR! errno -4075
+                npm ERR! EEXIST: file already exists, mkdir 'G:\Git\lsp-sample\node_modules.staging'
+                npm ERR! File exists: G:\Git\lsp-sample\node_modules.staging
+                npm ERR! Remove the existing file and try again, or run npm
+                npm ERR! with --force to overwrite files recklessly.
+            """.trimIndent()
+
+            output.lines().groupLines("npm ERR! ") shouldBe listOf(
+                "EEXIST: file already exists, mkdir 'G:\\Git\\lsp-sample\\node_modules.staging' " +
+                    "File exists: G:\\Git\\lsp-sample\\node_modules.staging " +
+                    "Remove the existing file and try again, or run npm " +
+                    "with --force to overwrite files recklessly."
+            )
+        }
     }
 })