Fix file path expansion for subpackages in go test output. #1651
Conversation
|
|
|
@ramya-rao-a Is there anything else needed here? |
src/testUtils.ts
Outdated
| const result = line.match(packageResultLineRE); | ||
| if (result) { | ||
| const packageNameArr = result[2].split('/'); | ||
| const baseDir = path.join(workspaceRoot, ...packageNameArr.slice(1)); |
There was a problem hiding this comment.
Using workspaceRoot doesn't work always here.
For example, assume I have opened the golang example repo in vscode and assume that a test in the stringutil package is failing
workspaceRoot will be /Users/myGopath/src/github.com/golang/example
packageNameArr after the slice will be [golang, example, stringutil]
baseDir becomes /Users/myGopath/src/github.com/golang/example/golang/example/stringutil which is wrong
currentGoWorkspace variable on the other hand has what you need. So use that instead
`const baseDir = path.join(currentGoWorkspace, ...packageNameArr);
Note that currentGoWorkspace can be undefined when we cannot determine the gopath or if current file doesn't not belong to current workspace
src/testUtils.ts
Outdated
| const packageNameArr = result[2].split('/'); | ||
| const baseDir = path.join(workspaceRoot, ...packageNameArr.slice(1)); | ||
| testResultLines.forEach(line => outputChannel.appendLine(expandFilePathInOutput(line, baseDir))); | ||
| testResultLines.splice(0); |
There was a problem hiding this comment.
this is to reset the array?
There was a problem hiding this comment.
Yes. It removes the elements without allocating a new array.
src/testUtils.ts
Outdated
| if (result) { | ||
| const packageNameArr = result[2].split('/'); | ||
| const baseDir = path.join(workspaceRoot, ...packageNameArr.slice(1)); | ||
| testResultLines.forEach(line => outputChannel.appendLine(expandFilePathInOutput(line, baseDir))); |
There was a problem hiding this comment.
For any which reason, in case go test changes the way it outputs result, then we will end up not showing anything in the output channel. I would prefer to pull the printing to the output channel out of the if block here
There was a problem hiding this comment.
I've fixed this by emitting any remaining lines at the end of the test output. If the regex doesn't match because the go test output has changed, this should emit all the results at the end. It's not ideal since none of the results will be visible until all of the tests have finished, but there's not much else I can do.
- Use the current Go workspace to resolve the full path to packages. - Don't expand file paths if the current Go workspace is not defined. - If there are any remaining buffered test result lines, emit them at the end.
|
Thanks! |
This fixes #1626 by using the package name from the go test output combined with the workspace root folder path to expand filenames to their correct path. go test itself buffers all test output in a package until all the tests in a package have completed, so adding our own per-package buffering does not have a visible effect on the latency of test reporting.