Improve the quality of the error message for minitest (#79)

rwx-research · Aug 28, 2024 · 34dc5f7 · 34dc5f7
1 parent 693f21b
commit 34dc5f7
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 14 deletions.
diff --git a/.mint/ci.yaml b/.mint/ci.yaml
@@ -12,7 +12,7 @@ on:
         branch: ${{ event.git.branch }}
 
 concurrency-pools:
-  - id: rwx-research/captain:ci:${{ init.commit-sha }}:${{ init.trigger }}
+  - id: rwx-research/captain:ci:${{ init.branch }}:${{ init.trigger }}
     if: ${{ init.branch != "main" }}
     capacity: 1
     on-overflow: cancel-running

diff --git a/internal/parsing/.snapshots/RubyMinitestParser Parse parses the sample file b/internal/parsing/.snapshots/RubyMinitestParser Parse parses the sample file
@@ -8,11 +8,11 @@
     "status": {
       "kind": "failed"
     },
-    "tests": 10,
+    "tests": 11,
     "otherErrors": 0,
     "retries": 0,
     "canceled": 0,
-    "failed": 3,
+    "failed": 4,
     "pended": 0,
     "quarantined": 0,
     "skipped": 2,
@@ -63,10 +63,10 @@
         },
         "status": {
           "kind": "failed",
-          "message": "RuntimeError: uh oh...",
+          "message": "RuntimeError: uh oh",
           "exception": "RuntimeError",
           "backtrace": [
-            "/Users/kylekthompson/src/captain-examples/minitest/test/failing_test.rb:9"
+            "/Users/kylekthompson/src/captain-examples/minitest/test/failing_test.rb:9:in `test_raises'"
           ]
         }
       }
@@ -88,10 +88,35 @@
         },
         "status": {
           "kind": "failed",
-          "message": "RWX::Example::MyError: uh oh...",
+          "message": "RWX::Example::MyError: uh oh",
           "exception": "RWX::Example::MyError",
           "backtrace": [
-            "/Users/kylekthompson/src/captain-examples/minitest/test/failing_test.rb:13"
+            "/Users/kylekthompson/src/captain-examples/minitest/test/failing_test.rb:13:in `test_raises_custom'"
+          ]
+        }
+      }
+    },
+    {
+      "name": "TestFailing#test_assert_equal_fails",
+      "lineage": [
+        "TestFailing",
+        "test_assert_equal_fails"
+      ],
+      "location": {
+        "file": "test/failing_test.rb",
+        "line": 25
+      },
+      "attempt": {
+        "durationInNanoseconds": 35000,
+        "meta": {
+          "assertions": 1
+        },
+        "status": {
+          "kind": "failed",
+          "message": "Expected: 2\n  Actual: 1",
+          "exception": "Minitest::Assertion",
+          "backtrace": [
+            "/Users/kylekthompson/src/captain-examples/minitest/test/rwx/example_test.rb:26"
           ]
         }
       }

diff --git a/internal/parsing/ruby_minitest_parser.go b/internal/parsing/ruby_minitest_parser.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"math"
 	"regexp"
+	"strings"
 	"time"
 
 	"github.com/rwx-research/captain-cli/internal/errors"
@@ -114,6 +115,11 @@ func (p RubyMinitestParser) Parse(data io.Reader) (*v1.TestResults, error) {
 	), nil
 }
 
+var (
+	rubyMinitestNewlineRegexp   = regexp.MustCompile(`\r?\n`)
+	rubyMinitestBacktraceRegexp = regexp.MustCompile("\\s{4}.+:in `.+'")
+)
+
 func (p RubyMinitestParser) NewFailedTestStatus(failure RubyMinitestFailure) v1.TestStatus {
 	failureMessage := failure.Message
 	failureException := failure.Type
@@ -122,10 +128,31 @@ func (p RubyMinitestParser) NewFailedTestStatus(failure RubyMinitestFailure) v1.
 		return v1.NewFailedTestStatus(failureMessage, failureException, nil)
 	}
 
-	location := rubyMinitestFailureLocationRegexp.FindStringSubmatch(*failure.Contents)
-	if len(location) < 2 {
-		return v1.NewFailedTestStatus(failureMessage, failureException, nil)
+	lines := rubyMinitestNewlineRegexp.Split(strings.TrimSpace(*failure.Contents), -1)[2:]
+
+	var failureBacktrace []string
+
+	if len(lines) > 0 {
+		failureMessageComponents := make([]string, 0)
+
+		for _, line := range lines {
+			if rubyMinitestBacktraceRegexp.Match([]byte(line)) {
+				failureBacktrace = append(failureBacktrace, strings.TrimSpace(line))
+			} else {
+				failureMessageComponents = append(failureMessageComponents, line)
+			}
+		}
+
+		constructedMessage := strings.Join(failureMessageComponents, "\n")
+		failureMessage = &constructedMessage
+	}
+
+	if failureBacktrace == nil {
+		location := rubyMinitestFailureLocationRegexp.FindStringSubmatch(*failure.Contents)
+		if len(location) >= 2 {
+			failureBacktrace = []string{location[1]}
+		}
 	}
 
-	return v1.NewFailedTestStatus(failureMessage, failureException, []string{location[1]})
+	return v1.NewFailedTestStatus(failureMessage, failureException, failureBacktrace)
 }
diff --git a/...rnal/targetedretries/.snapshots/RubyMinitestSubstitution works with a real file for Rails b/...rnal/targetedretries/.snapshots/RubyMinitestSubstitution works with a real file for Rails
@@ -1,5 +1,5 @@
 ([]map[string]string) (len=1) {
   (map[string]string) (len=1) {
-    (string) (len=5) "tests": (string) (len=75) "'test/failing_test.rb:4' 'test/failing_test.rb:8' 'test/failing_test.rb:12'"
+    (string) (len=5) "tests": (string) (len=101) "'test/failing_test.rb:4' 'test/failing_test.rb:8' 'test/failing_test.rb:12' 'test/failing_test.rb:25'"
   }
 }
diff --git a/.../targetedretries/.snapshots/RubyMinitestSubstitution works with a real file for non-Rails b/.../targetedretries/.snapshots/RubyMinitestSubstitution works with a real file for non-Rails
@@ -1,4 +1,8 @@
-([]map[string]string) (len=3) {
+([]map[string]string) (len=4) {
+  (map[string]string) (len=2) {
+    (string) (len=4) "file": (string) (len=20) "test/failing_test.rb",
+    (string) (len=4) "name": (string) (len=23) "test_assert_equal_fails"
+  },
   (map[string]string) (len=2) {
     (string) (len=4) "file": (string) (len=20) "test/failing_test.rb",
     (string) (len=4) "name": (string) (len=10) "test_fails"

diff --git a/test/fixtures/minitest.xml b/test/fixtures/minitest.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuites>
-  <testsuite name="TestFailing" filepath="test/failing_test.rb" skipped="0" failures="1" errors="2" tests="3" assertions="1" time="9.200000204145908e-05">
+  <testsuite name="TestFailing" filepath="test/failing_test.rb" skipped="0" failures="2" errors="2" tests="4" assertions="2" time="0.00012700003571808338">
     <testcase name="test_fails" lineno="4" classname="TestFailing" assertions="1" time="2.3000058718025684e-05" file="test/failing_test.rb">
     <failure type="Minitest::Assertion" message="Expected false to be truthy.">
 Failure:
@@ -24,6 +24,14 @@ RWX::Example::MyError: uh oh
     /Users/kylekthompson/src/captain-examples/minitest/test/failing_test.rb:13:in `test_raises_custom'
     </error>
     </testcase>
+    <testcase name="test_assert_equal_fails" lineno="25" classname="TestFailing" assertions="1" time="3.50000336766243e-05" file="test/failing_test.rb">
+    <failure type="Minitest::Assertion" message="Expected: 2...">
+Failure:
+test_assert_equal_fails(Minitest::Result) [/Users/kylekthompson/src/captain-examples/minitest/test/rwx/example_test.rb:26]:
+Expected: 2
+  Actual: 1
+    </failure>
+    </testcase>
   </testsuite>
   <testsuite name="RWX::TestExample" filepath="test/rwx/example_test.rb" skipped="0" failures="0" errors="0" tests="4" assertions="4" time="1.5039960000431165">
     <testcase name="test_is_slow" lineno="25" classname="RWX::TestExample" assertions="1" time="1.503885000012815" file="test/rwx/example_test.rb">