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.testFlags doesn't pass items before -args to debug binaries #1636

Closed
mgabeler-lee-6rs opened this issue Jul 21, 2021 · 11 comments
Closed
Assignees
Labels
debug/config Issues for config discrepancies b/n settings.json, launch.json, launch with F5, run test, debug test Debug Issues related to the debugging functionality of the extension. Go Companion Issues relating to the Go Companion extension NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@mgabeler-lee-6rs
Copy link

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

  • Run go version to get version of Go from the VS Code integrated terminal.
    • go version go1.16.6 linux/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
    • golang.org/x/tools/gopls v0.7.0
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
    • 1.58.2 c3f126316369cd610563c75b1b1725e0679adfb3 x64
  • Check your installed extensions to get the version of the VS Code Go extension
    • 0.26.0
  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.
Checking configured tools....
GOBIN: undefined
toolsGopath: 
gopath: /home/mgl/src/go
GOROOT: /usr/lib/go-1.16
PATH: /home/mgl/.nodenv/shims:/usr/lib/ccache:/home/mgl/src/mgl-incident-tools/bin:/home/mgl/bin:/home/mgl/.local/bin:/home/mgl/.nodenv/bin:/home/mgl/src/go/bin:/sbin:/usr/local/sbin:/usr/sbin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:/home/mgl/.npm/bin

   gopkgs: /home/mgl/src/go/bin/gopkgs installed
   go-outline: /home/mgl/src/go/bin/go-outline installed
   gotests: /home/mgl/src/go/bin/gotests installed
   gomodifytags: gomodifytags not installed
   impl: /home/mgl/src/go/bin/impl installed
   goplay: /home/mgl/src/go/bin/goplay installed
   dlv: /home/mgl/src/go/bin/dlv installed
   dlv-dap: dlv-dap not installed
   staticcheck: /home/mgl/src/go/bin/staticcheck installed
   gopls: /home/mgl/src/go/bin/gopls installed

Share the Go related settings you have added/edited

Run Preferences: Open Settings (JSON) command to open your settings.json file.
Share all the settings with the go. or ["go"] or gopls prefixes.

  "go.useLanguageServer": true,
  "go.gotoSymbol.includeImports": true,
  "go.buildOnSave": "workspace",
  "go.lintOnSave": "workspace",
  "go.vetOnSave": "workspace",
  "go.testOnSave": true,
  "go.testTimeout": "10s",
  "go.delveConfig": {
    "dlvLoadConfig": {
      "followPointers": true,
      "maxVariableRecurse": 1,
      "maxStringLen": 1024,
      "maxArrayValues": 1024,
      "maxStructFields": -1
    },
    "apiVersion": 2,
    "showGlobalVariables": false
  },
  "go.testFlags": [
    "-v"
  ],
  "go.toolsManagement.autoUpdate": true,

Describe the bug

This is basically a re-hash / follow-up to old issue microsoft/vscode-go#2115

If you set go.testFlags to ["-v"], then you get verbose test output when running tests but not debugging them. If you set it to ["-args","-test.v"] then you get verbose output when debugging but not when running. You have to combine the two via ["-v", "-args", "-test.v"] to get both behaviors.

This is quite confusing. It seems that testFlags elements before -args ought to be passed to the debug binary with the test. prefix added, the same way go test does when non-debug running tests.

Steps to reproduce the behavior:

  1. Set go.testFlags to ["-v"]
  2. Select a test
  3. Set a breakpoint within it
  4. When the breakpoint hits, run ps auxww | grep test or such to see the running processes, observe that the test is run as /path/to/something.test -run ^TestName$, instead of as /path/to/something.test -run ^TestName$ -test.v
@gopherbot gopherbot added this to the Untriaged milestone Jul 21, 2021
@findleyr
Copy link
Member

I agree that this is confusing, though I'm not sure we can change the behavior at this point. We will discuss this in our triage meeting.

For reference, would you find a separate configuration option for test arguments to be equally confusing?

@findleyr findleyr added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jul 21, 2021
@mgabeler-lee-6rs
Copy link
Author

From my perspective, there's "flags for building the test binary", and "flags for running the test binary". I would like to be able to configure the latter once, and have them apply equally whether I'm running or debugging that binary. If having go.testBuildFlags and go.testRunFlags was the "easy" way to do that, it'd be fine with me. My only concern with that model would be the confusion of folks expecting -v to "work", and not realizing that it has to be 'spelled' -test.v.

Also, poking around, I'm seeing some extra fun behavior where go test ./package/ -args test.v ... doesn't print the verbose stuff, and I instead have to write it as ... -test.v=true. But when running a compiled test binary, -test.v works fine. After looking at what go test -v does for exec calls under strace, I ... think that go test is itself interpreting the -v arg to control output suppression, in addition to passing it to the test binary it builds & runs to control what output is generated.

Which makes me think that making the use case of "I want vscode-go to always run my tests with verbose enabled" work without arcane invocations may simply require a special case no matter what (absent any changes to go test itself, anyways).

go test ./package/ -args test.v behaving differently than go test ./package/ -args test.v=true ... just feels like a 🐛 though 🙂

@suzmue suzmue modified the milestones: Untriaged, Backlog Jul 22, 2021
@findleyr
Copy link
Member

We discussed this today, and consensus was that we can't change this behavior at this point: we can't break users who have configurations containing -args.

Also, poking around, I'm seeing some extra fun behavior where go test ./package/ -args test.v ... doesn't print the verbose stuff, and I instead have to write it as ... -test.v=true

I'm not seeing this behavior. I see -test.v and -test.v=true behaving identically. It would be very surprising if they behaved differently, given that this is a bool flag.

@mgabeler-lee-6rs
Copy link
Author

I'm not seeing this behavior

Hmm ... yeah, I think that was an artifact of caching, or more likely typos. Doing a more careful, isolated test (below), I'm finding that no variation on -test.v after -args produces verbose output from go test ./packagename, as it seems that, even if the test app emits the output, go test suppresses it.

So it seems that, to reliably get verbose test output with the extension, the only way to do it right now is to double-request it by using -v -args -test.v.

I guess the question is, should the extension replicate how go test converts "before the -args" flags to "after the -args" flags? I'm not sure that making it do so would be a breaking change? Certainly users would no longer be able to replicate the "inconsistent" behavior of only getting verbose output when they don't debug the tests, but I don't personally see the utility in that behavior 😉

To me, having the extension act like go test is desirable, as is having "Run test" and "Debug test" run the test with the same args. Having those two behave differently is, to me, the crux of the "bug" here.


More fiddly bits:

Setup I'm working with:

  1. go mod init example
  2. cat demo_test.go:
package example
import "testing"
func TestDemo(t *testing.T) {
    t.Log("verbose output")
}
  • go test -args -test.v -- produces verbose output
  • go test ./ -args -test.v -- does not produce verbose output (strace shows the test binary is producing it, but the test runner is suppressing it)

I think the difference here is, subtly, captured by the go help test output:

The first, called local directory mode, occurs when go test is invoked with no package arguments ...
In this mode, go test compiles the package sources and tests found in the current directory and then runs the resulting test binary ...

The second, called package list mode, occurs when go test is invoked with explicit package arguments ...
In this mode, go test compiles and tests each of the packages listed on the command line.
If a package test passes, go test prints only the final 'ok' summary line.
(emphasis mine)

Confusing, but documented 🤷‍♂️

@mgabeler-lee-6rs
Copy link
Author

I fell on my face with this again trying to use the "Go Companion" extension. With the normal Go extension, the test explorer can still at least run tests with:

	"go.testFlags": [
		"-short",
		"-v",
		"-benchtime=1x",
		"-args",
		"-test.v",
		"-test.benchtime=1x",
	],

But the Go companion gets completely broken by having -args in there, because it appends some additional args after that which are only valid after it, esp. -buildflags=... and -fullpath.

Can the code that handles the debug launches just "parse" go.testFlags and recognize the flags that are really flags to the test program and adapt (filter ones not applicable to test programs, add test. prefix to others) & pass them through? That seems like it would be simpler than trying to insert custom args before the -args, and making users specify things twice like this.

Not being able to have t.Log... output visible when debugging is super frustrating ☹️

@firelizzard18 firelizzard18 added the Go Companion Issues relating to the Go Companion extension label Dec 12, 2024
@firelizzard18
Copy link
Contributor

  • go test -args -test.v -- produces verbose output
  • go test ./ -args -test.v -- does not produce verbose output (strace shows the test binary is producing it, but the test runner is suppressing it)

If this is something that needs to be fixed, that needs to be fixed in Go, not this extension (or Go Companion). I will note that moving the package specifier to the end works: go test -args -test.v ./ has the same behavior as go test -args -test.v.

Can the code that handles the debug launches just "parse" go.testFlags

It's already doing that. Debug launches are handled with dlv test, which operates in a substantially different way from go test. I am extremely reluctant to add parsing beyond what is absolutely necessary since it will inevitably be an endless source of edge cases and bugs. Go Companion v0.0.4 will have two relevant changes: user flags (go.testFlags) are now tracked separately and always appended after the flags the extension needs, and debug launches will handle user flags better. Debug launches have to differentiate between build flags (which need to be passed separately) and test flags and the test flags must always be prefixed with test., so now it discards -args (because delve doesn't support that flag) and avoids double-prefixing test flags. I tested with the reproduction you posted earlier and it now works in both cases (normal runs and debug runs). Also Go Companion always implicitly passes -v; it uses -json which also sets -v. It also explicitly removes -v from the flag list passed to go test because of golang/go#70384 though for user flags instead of removing -v I just log a warning.

I pushed v0.0.4 as a git tag which should trigger a release, though I only just set up the actions for that so I'm not 100% sure they'll work.

@firelizzard18
Copy link
Contributor

To be clear, this is fixed by using Go Companion v0.0.4, not in vscode-go. Honestly I don't 100% follow the "doesn't pass items" part, but IIUC the issue you want solved is using -v and -test.v and A) not exploding and B) getting verbose logs, which I fixed: given the reproduction example you posted, it doesn't explode and you get verbose logs.

With respect to the "doesn't pass items" part, Go Companion definitely passes items when doing a normal test run, and I fixed the order so that they're passed after the flags the extension sets like -fullpath. During a debug run, all test flags must be passed as args. That's the only way you can pass test flags to delve. Build flags are split out and passed with --build-flags, but all test flags must be prefixed and passed after --, as in: dlv test --build-flags "-cover" -- -test.v. Your example was exploding because -test.args is not a valid flag, so I had to exclude it.

Additionally, Go Companion (and vscode-go's test explorer) always use the equivalent of -json/-v because without that I wouldn't be able to tell vscode when a test passes/fails/etc.

@mgabeler-lee-6rs
Copy link
Author

With respect to the "doesn't pass items" part

That's about vscode-go, not Go Companion. This ticket predates the existence of Go Companion I think :)

It sounds like Go Companion 0.4.0 will fix most of the problems here.

The one that's left that I'm not clear on is the "Debug test" overlay/inlay hint decorated atop a test function declaration. That is the one that required the most shenanigans to get to run with verbose output mode. Does/will Go Companion take over that debug launch as well? Having to go find a test in the test explorer in order to do its debug launch right is ... not fun UX. Esp. when I'm finding that the Go Companion, at least at 0.3.0, tends to miss entire multiple entire packages within my workspace (that's a separate issue, of course)

@firelizzard18
Copy link
Contributor

I assume you're referring to "debug test" as in this screenshot? VSCode calls those "code lenses" (point being if you say "code lens" we'll immediately know what you're talking about).

image

There are currently three different ways of running tests: the legacy code lenses (run test | debug test), vscode-go's test explorer, and Go Companion's test explorer. The two vscode-go implementations have substantially different code, but they also share code. So a lot of the weirdness (as far as inexplicably different behavior between the three) comes down to that. The plan is to replace everything with Go Companion's version (and pull it into vscode-go) once it's mature.

Does/will Go Companion take over that debug launch as well?

If you're using a preview version of vscode-go with default settings the code lenses will disappear entirely. If you want them back set "go.experiments": { "testExplorer": false }, or you can enable Go Companion's equivalent code lenses with "exp-vscode-go.testExplorer.codeLens": true (or "debug" for only the debug lenses). With Go Companion's code lenses, clicking "debug test" is functionally identical to clicking debug in the test explorer.

Having to go find a test in the test explorer in order to do its debug launch right is ... not fun UX.

There are two other options: Right click on ▷ and click "Debug Test", or hold down alt/option which should turn ▷ into which will start a debug session.

Go Companion, at least at 0.3.0, tends to miss entire multiple entire packages within my workspace

If you open an issue (ideally with a reproducer) I'll take a look. If your workspace has a single go.mod file at the root, that should not be happening. If you do have multiple go.mod files, that could be why; gopls tracks each module separately and Go Companion simply asks gopls for a list of tests. If you have some module that you haven't opened any files from, gopls won't know it exists (more or less) and won't include its tests. If that is what's happening it's still worth discussing in an issue.

@mgabeler-lee-6rs
Copy link
Author

I assume you're referring to "debug test" as in this screenshot?

Yes. I knew it had a name but was failing to remember it :)

with default settings the code lenses will disappear entirely ... enable Go Companion's equivalent code lenses ...

I'm just one person, but personally I find that code lens super useful. I'll give the preview version a try and see how having Go Companion take over the mentioned items goes. It sounds like it should be better & more consistent, thanks :)

@mgabeler-lee-6rs
Copy link
Author

mgabeler-lee-6rs commented Dec 12, 2024

If you open an issue (ideally with a reproducer) I'll take a look. ... If you do have multiple go.mod files, that could be why ... If you have some module that you haven't opened any files from

I've not worked out what any pattern is, so making a reproducer may be challenging, but I'll give it a go.

FWIW, the vscode workspace in which I experienced this was a multi-folder one. The folder I was/am working in is itself a multi-module go workspace. But the package I first noticed missing was the one from the active editor window.

To avoid confusing this issue any further, I'll try to narrow down this issue and make a new ticket if I can get anything coherent / reproducible.

Edit: figured it out: golang/go#70929

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug/config Issues for config discrepancies b/n settings.json, launch.json, launch with F5, run test, debug test Debug Issues related to the debugging functionality of the extension. Go Companion Issues relating to the Go Companion extension NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

6 participants