-
Notifications
You must be signed in to change notification settings - Fork 763
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
test: add automated test for debug adapter #137
Comments
Change https://golang.org/cl/240097 mentions this issue: |
golang.org/cl/239697 accidentally added the new dependency on util - to find the value of go.goroot, which broke the debug adapter that runs outside vscode. Made getBinPathWithPreferredGopath accept a preferredGoroot param, and renamed the function to reflect this change. We need to remove any dependency from src/debugAdapter to src to avoid a similar bug in the future. We need to have a test, using the built extension. Updates #137. Change-Id: I586438f1f391aaff0c0cdc806de1e9f3fd5deba9 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/240097 Reviewed-by: Rebecca Stambler <[email protected]>
Change https://golang.org/cl/240117 mentions this issue: |
golang.org/cl/239697 accidentally added the new dependency on util - to find the value of go.goroot, which broke the debug adapter that runs outside vscode. Made getBinPathWithPreferredGopath accept a preferredGoroot param, and renamed the function to reflect this change. We need to remove any dependency from src/debugAdapter to src to avoid a similar bug in the future. We need to have a test, using the built extension. Updates #137. Change-Id: I586438f1f391aaff0c0cdc806de1e9f3fd5deba9 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/240097 Reviewed-by: Rebecca Stambler <[email protected]> (cherry picked from commit b9d1bad) Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/240117
A smokes test that will ensure that most basic operations (start-up, shutdown, restart, set breakpoint, continue, etc) are working should be our top priority. We do not want to break the basics while adding some advanced feature or fixing an obscure bug. Once something basic is in place, we can look into a more expanded set of tests. There are some interesting test cases and a report of how vscode-go did in 2019 at https://github.com/go-delve/delve/blob/master/Documentation/api/ClientHowto.md#testing-the-client. |
Change https://golang.org/cl/259206 mentions this issue: |
Add basic debug adapter tests to check that basic programs can run using the vscode-debugadapter-testsupport package. Updates #137 Change-Id: Ida60da545058f229253a27dcc4d66e94f558d113 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/259206 Trust: Suzy Mueller <[email protected]> Trust: Hyang-Ah Hana Kim <[email protected]> Run-TryBot: Suzy Mueller <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Change https://golang.org/cl/259800 mentions this issue: |
These tests check that the debugger runs when launched with test configurations. Updates #137 Change-Id: Ie34f8a954fd27ef6abcd896ac23575429b50cd37 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/259800 Trust: Suzy Mueller <[email protected]> Run-TryBot: Suzy Mueller <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Since we are making the move to use the delve dap implementation, the current testing plan is to avoid making major changes to the old debug adapter in order to test. I have added some tests of the DAP implementation using the test support provided by microsoft/vscode-debugadapter. These tests should continue to be able to test the new debug adapter as well. |
@suzmue Since now we have the test framework, can we close this issue? (Or maybe close this after your pending cl to run the same tests for delve dap?) |
Change https://golang.org/cl/290511 mentions this issue: |
Since dlv dap communicates over a network connection, we must start it as a server. In order to have tests for both the old and new debug adapter, we add the necessary code for dlv-dap guarded by a boolean to determine which mode to run the tests in. Updates #137 Change-Id: I211addebb1e3a2df512147b1603a538186e2f061 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/290511 Trust: Suzy Mueller <[email protected]> Run-TryBot: Suzy Mueller <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Polina Sokolova <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
From microsoft/vscode-go#2382 by @lggomez:
This issue is to keep track and brainstorm about ideas on how to add unit test support to the debug adapter
As of today, the debug adapter is the only part of the extension without tests and the most prone to regressions, due to the fact that it relies on the debugger process to work, and we still lack a standarized set of fixtures to run smoke tests (and, additionally, an automated way to do so)
I think that we should be able to add coverage for any basic scenarios we want (forget about remote for now), but some changes may be required to achieve this. The obvious to me are these:
Decouple GoDebugSession.Delve into an interface so it can be mocked
Finding a way to 'record' real scenarios so we can use those as the test cases. This means being able to intercept the json-rpc2 client apicalls. I believe this would allow us to test the debug protocol request implementations to some extent; logging request I/O and using those would be enough for now
Extra (and harder): Make a runner that is able to create and communicate with a GoDebugSession instance, which is the main entry point and contains most of the debugger-related logic of the adapter. This implies simulating a basic debug adapter lifecycle for each desired scenario (run and exit, breakpoints, pause, etc)
As Go and Delve evolve this should be a priority in the medium to long term roadmap, as it makes the development process more resilient against big changes and refactors in this area
The text was updated successfully, but these errors were encountered: