Skip to content
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

[Go Tests] Non-compilable code does not get its FilePath expanded #522

Closed
oliverpool opened this issue Aug 13, 2020 · 8 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@oliverpool
Copy link

oliverpool commented Aug 13, 2020

What version of Go, VS Code & VS Code Go extension are you using?

  • go version go1.15 linux/amd64
  • code -v
1.47.2
17299e413d5590b14ab0340ea477cdd86ff13daf
x64
  • VS Code Go 0.16.1

Share the Go related settings you have added/edited

"go.useLanguageServer": true,

Describe the bug

When I run a test against a source which is not compilable (undefined struct), the location of the compilation error is not expanded to the fullpath of the code (and is hence not clickable).

Steps to reproduce the behavior:

Repo to reproduce: https://github.com/oliverpool/vscode-go-bugreport

  1. Create a go file which can't compile. main.go
package main

func main() {
	Hello{}
}
  1. Run the tests Go: Test package
  2. See Output:
Running tool: /usr/bin/go test -timeout 30s -coverprofile=/tmp/vscode-go1sM0GN/go-code-cover

# github.com/oliverpool/vscode-go-bugreport
./main.go:4:2: undefined: Hello

The ./main.go:4:2 is not expanded, hence not clickable.

  1. Create a test main_test.go
package main

func Test() {
	Hello{}
}
  1. Run the tests Go: Test package
  2. See Output:
Running tool: /usr/bin/go test -timeout 30s -coverprofile=/tmp/vscode-go1sM0GN/go-code-cover

# github.com/oliverpool/vscode-go-bugreport
/home/vscode-go-bugreport/main_test.go:3:1: wrong signature for Test, must be: func Test(t *testing.T)
FAIL	github.com/oliverpool/vscode-go-bugreport [setup failed]

/home/vscode-go-bugreport/main_test.go:3:1 is clickable (to quickly jump to the issue

@oliverpool
Copy link
Author

oliverpool commented Aug 13, 2020

When I run go test from the commandline, I get exactly the same output.

To fix this issue:

  • either the output of go test must be changed to show the fullpath when the compilation error is not in the the test file
  • or adjust the expandFilePathInOutput of this extension to consider this edge case

@oliverpool
Copy link
Author

NB: sometime the failing output does not have a column value (./renderer.go:227: undefined: Matrix)

I can't reproduce it on a small example, but here is the output of my CLI:

> go test -timeout 30s
# github.com/tdewolff/canvas/pdf [github.com/tdewolff/canvas/pdf.test]
./renderer.go:227:61: undefined: Matrix
FAIL	github.com/tdewolff/canvas/pdf [build failed]
> go test -timeout 30s -coverprofile=/tmp/vscode-go4exoSB/go-code-cover
# github.com/tdewolff/canvas/pdf [github.com/tdewolff/canvas/pdf.test]
./renderer.go:227: undefined: Matrix
FAIL	github.com/tdewolff/canvas/pdf [build failed]

So this is more likely a go test issue, but this could easily be accounted for in this extension.

@oliverpool
Copy link
Author

Please let me know, if you think that this issues should be reported (and fixed) directly in the go test tool.

@hyangah
Copy link
Contributor

hyangah commented Aug 13, 2020

@oliverpool thanks for reporting the issue.
Compilation error is unstructured and I think the output is even printed before go test has a chance to handle, so it's somewhat tricky for us to handle. (See, go test -json output doesn't capture the compilation error messages)
File an issue in https://golang.org/issues if there is already an open issue already.

In my opinion, diagnostics feature is a more reliable way to surface compilation/build errors than the failure messages from the test output. For example, if you use the language server(gopls), the error will be reported with the correct location info already, so users don't have to depend on the test output.

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/248737 mentions this issue: src/testUtils.ts: fix parsing of the compiler error file expansion

@hyangah
Copy link
Contributor

hyangah commented Aug 15, 2020

@oliverpool cl/248737 fixes the file location expansion issue when handling the build error output.

@hyangah hyangah added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 15, 2020
@hyangah hyangah added this to the v0.16.2 milestone Aug 15, 2020
@oliverpool
Copy link
Author

oliverpool commented Aug 15, 2020

This look good, thanks!

Using a regex is brittle, but the proposed /^\s*(\S+\.go):(\d+):/ should cover most cases (it covers at least mine :-):

  • main.go:123:
  • ./main.go:123:

It does not match filenames with a space, but they always bear ambiguity:

  • Failure in the file: main.go:123
  • ./my main file.go (allowed by the tools, but uncommon enough to not be supported here)

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/252118 mentions this issue: [release] src/testUtils.ts: fix parsing of the compiler error file expansion

gopherbot pushed a commit that referenced this issue Sep 1, 2020
…pansion

Build error output format can be different from the test output format.
Adjust the regex change in expandFilePathInOutput so it can capture
build errors that contain column numbers as well while capturing the
file path from test outputs such as

```
  TestB: b_test.go:6: test failed
```

The above line was incorrectly expanded in pre v0.16.0

pre v0.16.0: /^\s*(.+.go):(\d+):/
v0.16.0, v0.16.1: /\s+(\S+.go):(\d+):\s+/
this CL: /\s*(\S+\.go):(\d+):/

Fixes #522

Change-Id: Ifa8f07d3bb7c5aaa61d40dd29d9fae77d8ad0cbe
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/248737
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
(cherry picked from commit 1e4dbe2)
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/252118
TryBot-Result: kokoro <noreply+kokoro@google.com>
@golang golang locked and limited conversation to collaborators Sep 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants