Fix bug in terratest-log-parser that mixes result lines #899
Conversation
|
NOTE: build is failing because of #898 |
| @@ -140,6 +140,7 @@ func parseAndStoreTestOutput( | |||
|
|
|||
| case isStatusLine(data): | |||
| testName := getTestNameFromStatusLine(data) | |||
| previousTestName = testName | |||
There was a problem hiding this comment.
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.
| @@ -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.
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.
The previous logic didn't make sense, so this was switched to default which makes more sense.
| 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.
This is a more direct case guard for the specific case that this is trying to capture.
yorinasub17
force-pushed
the
yori-fix-terratestparser
branch
from
555ffd1 to
295980d
Compare
|
UPDATE: Rebased on |
|
@brikis98 Can I get another approval now that the build is passing on the rebase? |
|
Thanks for review! Merging this in now! |
This fixes two bugs in
terratest-log-parser:terratest-log-parserwas incorrectly handling result lines such that they leaked into the wrong test. You can see this behavior in the modifications made to the test fixtures.