From fdf68c206bba113d18558368c8f946cf66242efb Mon Sep 17 00:00:00 2001
From: shmck <shawn.j.mckay@gmail.com>
Date: Sat, 4 Apr 2020 09:48:52 -0700
Subject: [PATCH 1/5] refactor parser to capture multiple failures

---
 src/services/testRunner/parser.test.ts | 29 ++++++-----
 src/services/testRunner/parser.ts      | 68 ++++++++++++++++++++------
 2 files changed, 72 insertions(+), 25 deletions(-)

diff --git a/src/services/testRunner/parser.test.ts b/src/services/testRunner/parser.test.ts
index 4a85ca6d..b194c9b0 100644
--- a/src/services/testRunner/parser.test.ts
+++ b/src/services/testRunner/parser.test.ts
@@ -1,22 +1,20 @@
 import parser from './parser'
 
 describe('parser', () => {
-  test('should detect success', () => {
+  test('should pass single success', () => {
     const example = `
-1..2
+1..1
 ok 1 - Should pass
-ok 2 - Should also pass
 `
-    expect(parser(example)).toEqual({ ok: true })
+    expect(parser(example)).toEqual({ ok: true, fails: [] })
   })
-  test('should detect failure', () => {
+  test('should detect multiple successes', () => {
     const example = `
-1..3
+1..2
 ok 1 - Should pass
-not ok 2 - This one fails
-ok 3 - Also passes
+ok 2 - Should also pass
 `
-    expect(parser(example).ok).toBe(false)
+    expect(parser(example)).toEqual({ ok: true, fails: [] })
   })
   test('should detect failure if no tests passed', () => {
     const example = `
@@ -26,6 +24,15 @@ ok 3 - Also passes
 #  FAIL  __tests__/sum.test.js
 
 not ok 1 ● sum › should add two numbers together
+`
+    expect(parser(example).ok).toBe(false)
+  })
+  test('should detect single failure among successes', () => {
+    const example = `
+1..3
+ok 1 - Should pass
+not ok 2 - This one fails
+ok 3 - Also passes
 `
     expect(parser(example).ok).toBe(false)
   })
@@ -37,7 +44,7 @@ not ok 2 - First to fail
 ok 3 - Also passes
 not ok 4 - Second to fail
 `
-    expect(parser(example).message).toBe('First to fail')
+    expect(parser(example).fails).toEqual([{ message: 'First to fail' }, { message: 'Second to fail' }])
   })
 
   test('should parse mocha tap example', () => {
@@ -65,6 +72,6 @@ ok 3 sumItems should total numbers accurately
 # fail 1
 # skip 0
 `
-    expect(parser(example).message).toBe("sumItems shouldn't return NaN")
+    expect(parser(example).fails).toEqual([{ message: "sumItems shouldn't return NaN" }])
   })
 })
diff --git a/src/services/testRunner/parser.ts b/src/services/testRunner/parser.ts
index 1f45acd1..41821975 100644
--- a/src/services/testRunner/parser.ts
+++ b/src/services/testRunner/parser.ts
@@ -1,29 +1,69 @@
 interface ParserOutput {
   ok: boolean
-  message?: string
+  fails: Array<{ message: string; details?: string }>
 }
 
-const fail = /^not ok \d+\s(\-\s)?(.+)+$/
-const ok = /^ok/
+const r = {
+  fail: /^not ok \d+\s(\-\s)?(.+)+$/,
+  pass: /^ok/,
+  details: /^#\s{2}(.+)$/,
+}
+
+const detect = (type: 'fail' | 'pass' | 'details', text: string) => r[type].exec(text)
 
 const parser = (text: string): ParserOutput => {
   const lines = text.split('\n')
-  let hasPass = false
+
+  const result: ParserOutput = {
+    ok: false,
+    fails: [],
+  }
+
+  // temporary holder of error detail strings
+  let currentDetails: string | null = null
+
+  const addCurrentDetails = () => {
+    if (currentDetails) {
+      result.fails[result.fails.length - 1].details = currentDetails
+      currentDetails = null
+    }
+  }
+
   for (const line of lines) {
-    if (line.length) {
-      // parse failed test
-      const failRegex = fail.exec(line)
-      if (!!failRegex) {
-        return { ok: false, message: failRegex[2] }
+    if (!line.length) {
+      continue
+    }
+    // be optimistic! check for success
+    if (!result.ok && !result.fails.length) {
+      if (!!detect('pass', line)) {
+        result.ok = true
+        addCurrentDetails()
+        continue
       }
-      if (!hasPass) {
-        if (!!ok.exec(line)) {
-          hasPass = true
-        }
+    }
+
+    // check for failure
+    const isFail = detect('fail', line)
+    if (!!isFail) {
+      result.ok = false
+      result.fails.push({ message: isFail[2] })
+      addCurrentDetails()
+      continue
+    }
+
+    // check for error details
+    const isDetails = detect('details', line)
+    if (!!isDetails) {
+      const lineDetails: string = isDetails[2]
+      if (!currentDetails) {
+        currentDetails = lineDetails
+      } else {
+        // @ts-ignore ignore as it must be a string
+        currentDetails += `\n${lineDetails}`
       }
     }
   }
-  return { ok: hasPass }
+  return result
 }
 
 export default parser

From e61c14dfd3d7558c20ad72a069bb9366f32cedba Mon Sep 17 00:00:00 2001
From: shmck <shawn.j.mckay@gmail.com>
Date: Sat, 4 Apr 2020 09:54:41 -0700
Subject: [PATCH 2/5] capture details in test parser

---
 src/services/testRunner/parser.test.ts | 17 +++++++++++++++++
 src/services/testRunner/parser.ts      |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/services/testRunner/parser.test.ts b/src/services/testRunner/parser.test.ts
index b194c9b0..475fb5da 100644
--- a/src/services/testRunner/parser.test.ts
+++ b/src/services/testRunner/parser.test.ts
@@ -74,4 +74,21 @@ ok 3 sumItems should total numbers accurately
 `
     expect(parser(example).fails).toEqual([{ message: "sumItems shouldn't return NaN" }])
   })
+  test('should capture error details', () => {
+    const example = `
+not ok 1 package.json should have a valid "author" key
+#  AssertionError [ERR_ASSERTION]: no "author" key provided
+#      at Context.<anonymous> (test/packagejson.test.js:11:12)
+#      at processImmediate (internal/timers.js:439:21)
+# tests 1
+# pass 0
+# fail 1
+# skip 0
+`
+    const result = parser(example)
+    expect(result.fails[0].message).toBe('package.json should have a valid "author" key')
+    expect(result.fails[0].details).toBe(`AssertionError [ERR_ASSERTION]: no "author" key provided
+at Context.<anonymous> (test/packagejson.test.js:11:12)
+at processImmediate (internal/timers.js:439:21)`)
+  })
 })
diff --git a/src/services/testRunner/parser.ts b/src/services/testRunner/parser.ts
index 41821975..548161c2 100644
--- a/src/services/testRunner/parser.ts
+++ b/src/services/testRunner/parser.ts
@@ -54,7 +54,7 @@ const parser = (text: string): ParserOutput => {
     // check for error details
     const isDetails = detect('details', line)
     if (!!isDetails) {
-      const lineDetails: string = isDetails[2]
+      const lineDetails: string = isDetails[1].trim()
       if (!currentDetails) {
         currentDetails = lineDetails
       } else {
@@ -63,6 +63,7 @@ const parser = (text: string): ParserOutput => {
       }
     }
   }
+  addCurrentDetails()
   return result
 }
 

From 267f752634e78749e96443df946267d9c68c8da4 Mon Sep 17 00:00:00 2001
From: shmck <shawn.j.mckay@gmail.com>
Date: Sat, 4 Apr 2020 10:46:38 -0700
Subject: [PATCH 3/5] add test failure details

---
 src/services/testRunner/parser.test.ts | 47 +++++++++++++++++++++++++-
 src/services/testRunner/parser.ts      |  7 ++--
 2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/src/services/testRunner/parser.test.ts b/src/services/testRunner/parser.test.ts
index 475fb5da..607d42bb 100644
--- a/src/services/testRunner/parser.test.ts
+++ b/src/services/testRunner/parser.test.ts
@@ -74,7 +74,7 @@ ok 3 sumItems should total numbers accurately
 `
     expect(parser(example).fails).toEqual([{ message: "sumItems shouldn't return NaN" }])
   })
-  test('should capture error details', () => {
+  test('should capture single error details', () => {
     const example = `
 not ok 1 package.json should have a valid "author" key
 #  AssertionError [ERR_ASSERTION]: no "author" key provided
@@ -91,4 +91,49 @@ not ok 1 package.json should have a valid "author" key
 at Context.<anonymous> (test/packagejson.test.js:11:12)
 at processImmediate (internal/timers.js:439:21)`)
   })
+  test('should capture multiple error details', () => {
+    const example = `
+not ok 1 package.json should have a valid "author" key
+#  AssertionError [ERR_ASSERTION]: no "author" key provided
+#      at Context.<anonymous> (test/packagejson.test.js:11:12)
+#      at processImmediate (internal/timers.js:439:21)
+not ok 2 package.json should have a valid "description" key
+#  AssertionError [ERR_ASSERTION]: no "description" key provided
+# tests 1
+# pass 0
+# fail 1
+# skip 0
+`
+    const result = parser(example)
+    expect(result.fails[0].message).toBe('package.json should have a valid "author" key')
+    expect(result.fails[0].details).toBe(`AssertionError [ERR_ASSERTION]: no "author" key provided
+at Context.<anonymous> (test/packagejson.test.js:11:12)
+at processImmediate (internal/timers.js:439:21)`)
+    expect(result.fails[1].message).toBe('package.json should have a valid "description" key')
+    expect(result.fails[1].details).toBe(`AssertionError [ERR_ASSERTION]: no "description" key provided`)
+  })
+  test('should capture multiple error details between successes', () => {
+    const example = `
+ok 1 first passing test
+not ok 2 package.json should have a valid "author" key
+#  AssertionError [ERR_ASSERTION]: no "author" key provided
+#      at Context.<anonymous> (test/packagejson.test.js:11:12)
+#      at processImmediate (internal/timers.js:439:21)
+ok 3 some passing test
+not ok 4 package.json should have a valid "description" key
+#  AssertionError [ERR_ASSERTION]: no "description" key provided
+ok 5 some passing test
+# tests 1
+# pass 0
+# fail 1
+# skip 0
+`
+    const result = parser(example)
+    expect(result.fails[0].message).toBe('package.json should have a valid "author" key')
+    expect(result.fails[0].details).toBe(`AssertionError [ERR_ASSERTION]: no "author" key provided
+at Context.<anonymous> (test/packagejson.test.js:11:12)
+at processImmediate (internal/timers.js:439:21)`)
+    expect(result.fails[1].message).toBe('package.json should have a valid "description" key')
+    expect(result.fails[1].details).toBe(`AssertionError [ERR_ASSERTION]: no "description" key provided`)
+  })
 })
diff --git a/src/services/testRunner/parser.ts b/src/services/testRunner/parser.ts
index 548161c2..c2f70c55 100644
--- a/src/services/testRunner/parser.ts
+++ b/src/services/testRunner/parser.ts
@@ -23,8 +23,9 @@ const parser = (text: string): ParserOutput => {
   let currentDetails: string | null = null
 
   const addCurrentDetails = () => {
-    if (currentDetails) {
-      result.fails[result.fails.length - 1].details = currentDetails
+    const failLength: number = result.fails.length
+    if (currentDetails && !!failLength) {
+      result.fails[failLength - 1].details = currentDetails
       currentDetails = null
     }
   }
@@ -46,8 +47,8 @@ const parser = (text: string): ParserOutput => {
     const isFail = detect('fail', line)
     if (!!isFail) {
       result.ok = false
-      result.fails.push({ message: isFail[2] })
       addCurrentDetails()
+      result.fails.push({ message: isFail[2] })
       continue
     }
 

From 9b9c55f11df92e8125533e6a067a5216b0165b97 Mon Sep 17 00:00:00 2001
From: shmck <shawn.j.mckay@gmail.com>
Date: Sat, 4 Apr 2020 11:02:33 -0700
Subject: [PATCH 4/5] collect passing success as well

---
 src/services/testRunner/parser.test.ts | 33 +++++++++++++++-----------
 src/services/testRunner/parser.ts      | 27 +++++++++++----------
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/src/services/testRunner/parser.test.ts b/src/services/testRunner/parser.test.ts
index 607d42bb..d381dc75 100644
--- a/src/services/testRunner/parser.test.ts
+++ b/src/services/testRunner/parser.test.ts
@@ -6,7 +6,7 @@ describe('parser', () => {
 1..1
 ok 1 - Should pass
 `
-    expect(parser(example)).toEqual({ ok: true, fails: [] })
+    expect(parser(example)).toEqual({ ok: true, passed: [{ message: 'Should pass' }], failed: [] })
   })
   test('should detect multiple successes', () => {
     const example = `
@@ -14,7 +14,12 @@ ok 1 - Should pass
 ok 1 - Should pass
 ok 2 - Should also pass
 `
-    expect(parser(example)).toEqual({ ok: true, fails: [] })
+    const result = parser(example)
+    expect(result).toEqual({
+      ok: true,
+      passed: [{ message: 'Should pass' }, { message: 'Should also pass' }],
+      failed: [],
+    })
   })
   test('should detect failure if no tests passed', () => {
     const example = `
@@ -44,7 +49,7 @@ not ok 2 - First to fail
 ok 3 - Also passes
 not ok 4 - Second to fail
 `
-    expect(parser(example).fails).toEqual([{ message: 'First to fail' }, { message: 'Second to fail' }])
+    expect(parser(example).failed).toEqual([{ message: 'First to fail' }, { message: 'Second to fail' }])
   })
 
   test('should parse mocha tap example', () => {
@@ -72,7 +77,7 @@ ok 3 sumItems should total numbers accurately
 # fail 1
 # skip 0
 `
-    expect(parser(example).fails).toEqual([{ message: "sumItems shouldn't return NaN" }])
+    expect(parser(example).failed).toEqual([{ message: "sumItems shouldn't return NaN" }])
   })
   test('should capture single error details', () => {
     const example = `
@@ -86,8 +91,8 @@ not ok 1 package.json should have a valid "author" key
 # skip 0
 `
     const result = parser(example)
-    expect(result.fails[0].message).toBe('package.json should have a valid "author" key')
-    expect(result.fails[0].details).toBe(`AssertionError [ERR_ASSERTION]: no "author" key provided
+    expect(result.failed[0].message).toBe('package.json should have a valid "author" key')
+    expect(result.failed[0].details).toBe(`AssertionError [ERR_ASSERTION]: no "author" key provided
 at Context.<anonymous> (test/packagejson.test.js:11:12)
 at processImmediate (internal/timers.js:439:21)`)
   })
@@ -105,12 +110,12 @@ not ok 2 package.json should have a valid "description" key
 # skip 0
 `
     const result = parser(example)
-    expect(result.fails[0].message).toBe('package.json should have a valid "author" key')
-    expect(result.fails[0].details).toBe(`AssertionError [ERR_ASSERTION]: no "author" key provided
+    expect(result.failed[0].message).toBe('package.json should have a valid "author" key')
+    expect(result.failed[0].details).toBe(`AssertionError [ERR_ASSERTION]: no "author" key provided
 at Context.<anonymous> (test/packagejson.test.js:11:12)
 at processImmediate (internal/timers.js:439:21)`)
-    expect(result.fails[1].message).toBe('package.json should have a valid "description" key')
-    expect(result.fails[1].details).toBe(`AssertionError [ERR_ASSERTION]: no "description" key provided`)
+    expect(result.failed[1].message).toBe('package.json should have a valid "description" key')
+    expect(result.failed[1].details).toBe(`AssertionError [ERR_ASSERTION]: no "description" key provided`)
   })
   test('should capture multiple error details between successes', () => {
     const example = `
@@ -129,11 +134,11 @@ ok 5 some passing test
 # skip 0
 `
     const result = parser(example)
-    expect(result.fails[0].message).toBe('package.json should have a valid "author" key')
-    expect(result.fails[0].details).toBe(`AssertionError [ERR_ASSERTION]: no "author" key provided
+    expect(result.failed[0].message).toBe('package.json should have a valid "author" key')
+    expect(result.failed[0].details).toBe(`AssertionError [ERR_ASSERTION]: no "author" key provided
 at Context.<anonymous> (test/packagejson.test.js:11:12)
 at processImmediate (internal/timers.js:439:21)`)
-    expect(result.fails[1].message).toBe('package.json should have a valid "description" key')
-    expect(result.fails[1].details).toBe(`AssertionError [ERR_ASSERTION]: no "description" key provided`)
+    expect(result.failed[1].message).toBe('package.json should have a valid "description" key')
+    expect(result.failed[1].details).toBe(`AssertionError [ERR_ASSERTION]: no "description" key provided`)
   })
 })
diff --git a/src/services/testRunner/parser.ts b/src/services/testRunner/parser.ts
index c2f70c55..153f98ed 100644
--- a/src/services/testRunner/parser.ts
+++ b/src/services/testRunner/parser.ts
@@ -1,11 +1,12 @@
 interface ParserOutput {
   ok: boolean
-  fails: Array<{ message: string; details?: string }>
+  passed: Array<{ message: string }>
+  failed: Array<{ message: string; details?: string }>
 }
 
 const r = {
   fail: /^not ok \d+\s(\-\s)?(.+)+$/,
-  pass: /^ok/,
+  pass: /^ok \d+\s(\-\s)?(.+)+$/,
   details: /^#\s{2}(.+)$/,
 }
 
@@ -15,17 +16,18 @@ const parser = (text: string): ParserOutput => {
   const lines = text.split('\n')
 
   const result: ParserOutput = {
-    ok: false,
-    fails: [],
+    ok: true,
+    passed: [],
+    failed: [],
   }
 
   // temporary holder of error detail strings
   let currentDetails: string | null = null
 
   const addCurrentDetails = () => {
-    const failLength: number = result.fails.length
+    const failLength: number = result.failed.length
     if (currentDetails && !!failLength) {
-      result.fails[failLength - 1].details = currentDetails
+      result.failed[failLength - 1].details = currentDetails
       currentDetails = null
     }
   }
@@ -35,12 +37,11 @@ const parser = (text: string): ParserOutput => {
       continue
     }
     // be optimistic! check for success
-    if (!result.ok && !result.fails.length) {
-      if (!!detect('pass', line)) {
-        result.ok = true
-        addCurrentDetails()
-        continue
-      }
+    const isPass = detect('pass', line)
+    if (!!isPass) {
+      result.passed.push({ message: isPass[2].trim() })
+      addCurrentDetails()
+      continue
     }
 
     // check for failure
@@ -48,7 +49,7 @@ const parser = (text: string): ParserOutput => {
     if (!!isFail) {
       result.ok = false
       addCurrentDetails()
-      result.fails.push({ message: isFail[2] })
+      result.failed.push({ message: isFail[2].trim() })
       continue
     }
 

From e358c41d0c1e6c730103a6e9ccf368a7498e33d6 Mon Sep 17 00:00:00 2001
From: shmck <shawn.j.mckay@gmail.com>
Date: Sat, 4 Apr 2020 12:41:20 -0700
Subject: [PATCH 5/5] improve error formatting

---
 src/services/testRunner/formatOutput.ts | 15 +++++++++++++++
 src/services/testRunner/index.ts        | 12 +++++++-----
 src/services/testRunner/output.ts       | 13 +------------
 src/services/testRunner/parser.ts       |  2 +-
 4 files changed, 24 insertions(+), 18 deletions(-)
 create mode 100644 src/services/testRunner/formatOutput.ts

diff --git a/src/services/testRunner/formatOutput.ts b/src/services/testRunner/formatOutput.ts
new file mode 100644
index 00000000..279a5612
--- /dev/null
+++ b/src/services/testRunner/formatOutput.ts
@@ -0,0 +1,15 @@
+// @ts-ignore no declaration files
+import * as clc from 'cli-color'
+import { ParserOutput } from './parser'
+
+// TODO: implement better success ouput
+// export const formatSuccessOutput = (tap: ParserOutput): string => {}
+
+export const formatFailOutput = (tap: ParserOutput): string => {
+  let output = `'TESTS FAILED\n`
+  tap.failed.forEach((fail) => {
+    const details = fail.details ? `\n${fail.details}\n\n` : ''
+    output += `  ✘ ${fail.message}\n${details}`
+  })
+  return output
+}
diff --git a/src/services/testRunner/index.ts b/src/services/testRunner/index.ts
index 4dba1966..b6fec26e 100644
--- a/src/services/testRunner/index.ts
+++ b/src/services/testRunner/index.ts
@@ -4,6 +4,7 @@ import parser from './parser'
 import { debounce, throttle } from './throttle'
 import onError from '../sentry/onError'
 import { clearOutput, displayOutput } from './output'
+import { formatFailOutput } from './formatOutput'
 
 export interface Payload {
   stepId: string
@@ -52,11 +53,12 @@ const createTestRunner = (config: TestRunnerConfig, callbacks: Callbacks) => {
 
     const tap = parser(stdout || '')
     if (stderr) {
-      // failures also trigger stderr
+      // FAIL also trigger stderr
       if (stdout && stdout.length && !tap.ok) {
-        const message = tap.message ? tap.message : ''
-        callbacks.onFail(payload, message)
-        displayOutput(stdout)
+        const firstFailMessage = tap.failed[0].message
+        callbacks.onFail(payload, firstFailMessage)
+        const output = formatFailOutput(tap)
+        displayOutput(output)
         return
       } else {
         callbacks.onError(payload)
@@ -66,7 +68,7 @@ const createTestRunner = (config: TestRunnerConfig, callbacks: Callbacks) => {
       }
     }
 
-    // success!
+    // PASS
     if (tap.ok) {
       clearOutput()
       callbacks.onSuccess(payload)
diff --git a/src/services/testRunner/output.ts b/src/services/testRunner/output.ts
index ce988d5c..871a1de4 100644
--- a/src/services/testRunner/output.ts
+++ b/src/services/testRunner/output.ts
@@ -11,22 +11,11 @@ const getOutputChannel = (name: string): vscode.OutputChannel => {
 
 const outputChannelName = 'CodeRoad Output'
 
-const parseOutput = (text: string): string => {
-  let result = ''
-  for (const line of text.split(/\r?\n/)) {
-    if (line.match(/^#/) || line.match(/^not ok/)) {
-      result += line + '\n'
-    }
-  }
-  return result
-}
-
 export const displayOutput = (text: string) => {
   const channel = getOutputChannel(outputChannelName)
   channel.clear()
   channel.show(true)
-  const output = parseOutput(text)
-  channel.append(output)
+  channel.append(text)
 }
 
 export const clearOutput = () => {
diff --git a/src/services/testRunner/parser.ts b/src/services/testRunner/parser.ts
index 153f98ed..fc952f19 100644
--- a/src/services/testRunner/parser.ts
+++ b/src/services/testRunner/parser.ts
@@ -1,4 +1,4 @@
-interface ParserOutput {
+export interface ParserOutput {
   ok: boolean
   passed: Array<{ message: string }>
   failed: Array<{ message: string; details?: string }>