Open polinasok opened 4 years ago
Thanks @polinasok for putting up all the details.
The tricky part in the presented example is that the config in the launch.json
is not really compatible with general debug test
execution. I think we can consider
1) allow users to select one of the launch configurations and a default one, and
2) plumb "go.testFlags"
when mode
is test so users don't need to write launch.json
like the example at all.
What do you mean by "not really compatible"?
Do you mean "as the default one"? I am not sure what it would mean to pick two launch configurations.
I think many users want to use launch.json
and might not even know about go.testFlags
.
@polinasok The launch configuration has the following -
"args": [
"-test.run",
"TestA",
"-test.v"
]
This is not suitable for general 'debug test' codelens launch configuration - the main purpose of debug test codelen is to automatically determine the -test.run
flag value.
Users may have multiple launch configurations in the launch.json
- for example, I have launch configurations to run my binaries no matter what files are open. That's not suitable for the use with the debug test
codelens or even a regular testing. We can choose to present all we found from the launch.json and the default one and let users to choose.
I sent multiple users to the launch.json
path and let them specify the args there in addition to go.testFlags
. :-)
I see what you mean. I included -test.run
to match the individual function setup from run/debug test
for demonstration purposes. I figured one might wonder if they should match the choice from run/debug test
for this to work, without worrying about the hidden implementation details. But it does not work either way. If the two configurations had been combined as-is, we would have gotten an error like go test: run flag may be set only once
, which we do not. I added a comment to highlight this above.
Interestingly, adding ["-test.run", "TestA"]
to go.testFlags
does lead to an error with run test
and run file tests
while adding ["-run", "TestA"]
is a no-op for both. And adding either of those works with run package tests
, which doesn't use its own -run
flag by default.
If we do support this feature, it is up to us how to combine the two sources of -test.run
, isn't it? We could combine everything as-is and channel the duplication errors or apply some clever logic to make sense of the choices and avoid unnecessary errors (e.g. expect them to match, or one be a superset of the other, or expect that launch configuration has none specified, or always ignore the one specified in launch configuration favor of what run/debug test
or run file tests
adds). Moreover, we need to sanity check mode and the program in the launch configuration as well. A user might accidentally select something not related to their test at all, in which case we could just fallback to the default configuration.
Whatever we do, we just need to make sure it is documented and also flagged via some kind of warnings/logging, so if the behavior is unexpected, the user can get an explanation without having to search forums or file issues.
Related issue: #128 The many ways to specify build/test flags/tags that sometimes override each other and sometimes are ignored is causing issues in all directions. In the short-term, whatever the behavior is we need to be very careful about documenting it (#860).
In the long-term, we have the option of revisiting this alltogether with dlv-dap. New adapter, new flags. Should we? If according to https://github.com/microsoft/vscode-go/issues/3185#issuecomment-620399442, the options in launch.json
were added as a workaround, should they be dropped now that we can channel the settings from the extension? On the other hand, many users are now relying on launch.json
as a centralized place to keep all their configurations. It's direct mapping to launch/attach request args is very convenient.
One disadvantage of launch.json
is users have to configure this per workspace, unlike settings.json that can be defined for user profile and apply to all projects/workspaces.
Change https://golang.org/cl/366915 mentions this issue: docs/debugging: include logpoints, remove stop conditions, revise faqs
codelens also doesn't support the new 'asRoot' launch option that allows debugging and testing launch configurations to run as root.
also finding this limitation frustrating, I need to pass args
to the test binary, I can achieve it with launch.json
but then the code lens and gutter shortcuts are useless, as those don't care about launch.json
and there's no way to modify the args passed to the binary, just to the build command (via go.testFlags
)
Why are you passing args to a test binary? That sounds like something that would be better done with a main func/package.
Users complain that the handy
run test | debug test
links above their test functions ignore the flags they set in their launch configuration (example, example, example). They can fallback onsettings.json
usinggo.testFlags
,go.delveConfig
, etc, but that is not obvious or intuitive and has surprised users with additional limitations fordebug test
vsrun test
(e.g. #1636, https://github.com/microsoft/vscode-go/issues/2894#issuecomment-554303825, https://github.com/microsoft/vscode-go/issues/2115#issuecomment-438137710).According to @ramya-rao-a, a while back there was no way to feed current selected launch configuration into the codelens, so another set of dlv-related settings was supported as an alternative. Since then a new mechanism to support this could have been introduced, so we should investigate what is possible now.
Below is a quick way to reproduce the current behavior:
Test function:
Selected launch configuration
Debug with
▷ Start Debugging
orRun > Start Debugging (F5)
Uses
launch.json
configuration, so the test passes.Debug with
debug test
Doesn't use
launch.json
configuration, so the test fails.Run with
Run > Run Without Debugging (^F5)
Uses
launch.json
configuration, so the test passes. (Note that this actually goes through the debug adapter, which launches dlv - see #336)Run with
run test
Doesn't use
launch.json
configuration, so the test fails. (Note that this usesgo test
and bypasses dlv - see #336)Adding flags to
settings.json
run test
now passes, butdebug test
behavior is unchanged.run test
passes, but only prints "ok", no details.debug test
works as expected.Combining these makes both work as expected - pass and print verbose details.