-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bug in terratest-log-parser that mixes result lines #899
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
=== RUN TestBasicExample | ||
--- FAIL: TestBasicExample (0.00s) | ||
integration_test.go:10: | ||
integration_test.go:10: | ||
Error Trace: integration_test.go:10 | ||
Error: Expected value not to be nil. | ||
Test: TestBasicExample |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
=== RUN TestPanicExample | ||
--- FAIL: TestPanicExample (0.00s) | ||
integration_test.go:14: | ||
integration_test.go:14: | ||
Error Trace: integration_test.go:14 | ||
Error: Expected value not to be nil. | ||
Test: TestPanicExample |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
=== RUN TestRealWorldExample | ||
--- FAIL: TestRealWorldExample (0.00s) | ||
integration_test.go:18: | ||
integration_test.go:18: | ||
Error Trace: integration_test.go:18 | ||
Error: Expected value not to be nil. | ||
Test: TestRealWorldExample |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
=== RUN TestIntegrationBasicExample | ||
=== PAUSE TestIntegrationBasicExample | ||
=== RUN TestIntegrationFailingExample | ||
=== PAUSE TestIntegrationFailingExample | ||
=== RUN TestIntegrationPanicExample | ||
=== PAUSE TestIntegrationPanicExample | ||
=== CONT TestIntegrationBasicExample | ||
=== CONT TestIntegrationPanicExample | ||
=== CONT TestIntegrationFailingExample | ||
=== CONT TestIntegrationBasicExample | ||
integration_test.go:57: | ||
Error Trace: integration_test.go:57 | ||
Error: Should be true | ||
Test: TestIntegrationBasicExample | ||
--- PASS: TestIntegrationPanicExample (0.00s) | ||
--- PASS: TestIntegrationFailingExample (0.00s) | ||
--- FAIL: TestIntegrationBasicExample (0.00s) | ||
FAIL | ||
FAIL github.com/gruntwork-io/terratest/modules/logger/parser 1.589s | ||
FAIL |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
=== RUN TestIntegrationBasicExample | ||
=== PAUSE TestIntegrationBasicExample | ||
=== CONT TestIntegrationBasicExample | ||
=== CONT TestIntegrationBasicExample | ||
integration_test.go:57: | ||
Error Trace: integration_test.go:57 | ||
Error: Should be true | ||
Test: TestIntegrationBasicExample | ||
--- FAIL: TestIntegrationBasicExample (0.00s) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
=== RUN TestIntegrationFailingExample | ||
=== PAUSE TestIntegrationFailingExample | ||
=== CONT TestIntegrationFailingExample | ||
--- PASS: TestIntegrationFailingExample (0.00s) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
=== RUN TestIntegrationPanicExample | ||
=== PAUSE TestIntegrationPanicExample | ||
=== CONT TestIntegrationPanicExample | ||
--- PASS: TestIntegrationPanicExample (0.00s) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<testsuites> | ||
<testsuite tests="3" failures="1" time="1.589" name="github.com/gruntwork-io/terratest/modules/logger/parser"> | ||
<properties> | ||
<property name="go.version" value="go1.16.3"></property> | ||
</properties> | ||
<testcase classname="parser" name="TestIntegrationBasicExample" time="0.000"> | ||
<failure message="Failed" type=""> integration_test.go:57: </failure> | ||
</testcase> | ||
<testcase classname="parser" name="TestIntegrationFailingExample" time="0.000"></testcase> | ||
<testcase classname="parser" name="TestIntegrationPanicExample" time="0.000"></testcase> | ||
</testsuite> | ||
</testsuites> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- PASS: TestIntegrationPanicExample (0.00s) | ||
--- PASS: TestIntegrationFailingExample (0.00s) | ||
--- FAIL: TestIntegrationBasicExample (0.00s) | ||
FAIL | ||
FAIL github.com/gruntwork-io/terratest/modules/logger/parser 1.589s | ||
FAIL |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,7 +41,7 @@ func SpawnParsers(logger *logrus.Logger, reader io.Reader, outputDir string) { | |
var ( | ||
regexResult = regexp.MustCompile(`--- (PASS|FAIL|SKIP): (.+) \((\d+\.\d+)(?: ?seconds|s)\)`) | ||
regexStatus = regexp.MustCompile(`=== (RUN|PAUSE|CONT)\s+(.+)`) | ||
regexSummary = regexp.MustCompile(`^(ok|FAIL)\s+([^ ]+)\s+(?:(\d+\.\d+)s|\(cached\)|(\[\w+ failed]))(?:\s+coverage:\s+(\d+\.\d+)%\sof\sstatements(?:\sin\s.+)?)?$`) | ||
regexSummary = regexp.MustCompile(`(^FAIL$)|(^(ok|FAIL)\s+([^ ]+)\s+(?:(\d+\.\d+)s|\(cached\)|(\[\w+ failed]))(?:\s+coverage:\s+(\d+\.\d+)%\sof\sstatements(?:\sin\s.+)?)?$)`) | ||
regexPanic = regexp.MustCompile(`^panic:`) | ||
) | ||
|
||
|
@@ -140,6 +140,7 @@ func parseAndStoreTestOutput( | |
|
||
case isStatusLine(data): | ||
testName := getTestNameFromStatusLine(data) | ||
previousTestName = testName | ||
logWriter.writeLog(logger, testName, data) | ||
|
||
case strings.HasPrefix(data, "Test"): | ||
|
@@ -149,12 +150,11 @@ func parseAndStoreTestOutput( | |
// This must be modified when `logger.DoLog` changes. | ||
vals := strings.Split(data, " ") | ||
testName := vals[0] | ||
logWriter.writeLog(logger, testName, data) | ||
previousTestName = testName | ||
logWriter.writeLog(logger, testName, data) | ||
|
||
case isIndented && previousTestName != "summary": | ||
// In a test result block, so collect the line into all the test results we have seen so far. | ||
// Note that previousTestName would only be set to summary if we saw a panic line. | ||
case isIndented && isResultLine(data): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a more direct case guard for the specific case that this is trying to capture. |
||
// In a nested test result block, so collect the line into all the test results we have seen so far. | ||
for _, marker := range testResultMarkers { | ||
logWriter.writeLog(logger, marker.TestName, data) | ||
} | ||
|
@@ -164,13 +164,15 @@ func parseAndStoreTestOutput( | |
previousTestName = "summary" | ||
logWriter.writeLog(logger, "summary", data) | ||
|
||
case isResultLine(data): | ||
// We ignore result lines, because that is handled specially below. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes the bug where the results lines leak into unrelated test logs. The result lines were ending up in the base case right below this because there was no case guard that handled the result lines separately. |
||
|
||
case previousTestName != "": | ||
// Base case: roll up to the previous test line, if it exists. | ||
// Handles case where terratest log has entries with newlines in them. | ||
logWriter.writeLog(logger, previousTestName, data) | ||
|
||
case !isResultLine(data): | ||
// Result Lines are handled below | ||
default: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous logic didn't make sense, so this was switched to |
||
logger.Warnf("Found test line that does not match known cases: %s", data) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the fix for the lost error trace. The error trace line was happening right after a
=== CONT TESTNAME
line, which is a status line. Now we roll up any unmarked test lines that show up after a status line to the test that relates to the status line.