From 4be75bec5910d17c119acfd79d9017078e8ae3e3 Mon Sep 17 00:00:00 2001 From: polinasok <51177946+polinasok@users.noreply.github.com> Date: Thu, 28 May 2020 14:01:51 -0700 Subject: [PATCH] service/dap: Add buildFlags error checking and tests in launch requests (#2056) * service/dap: Add error checking and tests for buildFlags in launch requests * Clarify with comments and better naming * Undo redundant support.go changes * Undo redundant support.go changes (attempt #2) Co-authored-by: Polina Sokolova --- _fixtures/buildflagtest.go | 14 ++++++++++++++ service/dap/server.go | 13 ++++++++++--- service/dap/server_test.go | 33 +++++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 _fixtures/buildflagtest.go diff --git a/_fixtures/buildflagtest.go b/_fixtures/buildflagtest.go new file mode 100644 index 00000000..92ae8c5e --- /dev/null +++ b/_fixtures/buildflagtest.go @@ -0,0 +1,14 @@ +package main + +import "fmt" + +// To be set via +// go build -ldflags '-X main.Hello=World' +var Hello string + +func main() { + if len(Hello) < 1 { + panic("global main.Hello not set via build flags") + } + fmt.Printf("Hello %s!\n", Hello) +} diff --git a/service/dap/server.go b/service/dap/server.go index eb140818..c2fd0f20 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -402,9 +402,16 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) { return } - buildFlags, ok := request.Arguments["buildFlags"].(string) - if !ok { - buildFlags = "" + buildFlags := "" + buildFlagsArg, ok := request.Arguments["buildFlags"] + if ok { + buildFlags, ok = buildFlagsArg.(string) + if !ok { + s.sendErrorResponse(request.Request, + FailedToContinue, "Failed to launch", + fmt.Sprintf("'buildFlags' attribute '%v' in debug configuration is not a string.", buildFlagsArg)) + return + } } switch mode { diff --git a/service/dap/server_test.go b/service/dap/server_test.go index d020f16e..ff86808b 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -345,7 +345,8 @@ func runDebugSession(t *testing.T, client *daptest.Client, launchRequest func()) func TestLaunchDebugRequest(t *testing.T) { runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { - // We reuse the harness that builds, but ignore the actual binary. + // We reuse the harness that builds, but ignore the built binary, + // only relying on the source to be built in response to LaunchRequest. runDebugSession(t, client, func() { // Use the default output directory. client.LaunchRequestWithArgs(map[string]interface{}{ @@ -357,7 +358,8 @@ func TestLaunchDebugRequest(t *testing.T) { func TestLaunchTestRequest(t *testing.T) { runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { runDebugSession(t, client, func() { - // We reuse the harness that builds, but ignore the actual binary. + // We reuse the harness that builds, but ignore the built binary, + // only relying on the source to be built in response to LaunchRequest. fixtures := protest.FindFixturesDir() testdir, _ := filepath.Abs(filepath.Join(fixtures, "buildtest")) client.LaunchRequestWithArgs(map[string]interface{}{ @@ -366,6 +368,10 @@ func TestLaunchTestRequest(t *testing.T) { }) } +// Tests that 'args' from LaunchRequest are parsed and passed to the target +// program. The target program exits without an error on success, and +// panics on error, causing an unexpected StoppedEvent instead of +// Terminated Event. func TestLaunchRequestWithArgs(t *testing.T) { runTest(t, "testargs", func(client *daptest.Client, fixture protest.Fixture) { runDebugSession(t, client, func() { @@ -376,6 +382,22 @@ func TestLaunchRequestWithArgs(t *testing.T) { }) } +// Tests that 'buildFlags' from LaunchRequest are parsed and passed to the +// compiler. The target program exits without an error on success, and +// panics on error, causing an unexpected StoppedEvent instead of +// TerminatedEvent. +func TestLaunchRequestWithBuildFlags(t *testing.T) { + runTest(t, "buildflagtest", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSession(t, client, func() { + // We reuse the harness that builds, but ignore the built binary, + // only relying on the source to be built in response to LaunchRequest. + client.LaunchRequestWithArgs(map[string]interface{}{ + "mode": "debug", "program": fixture.Source, + "buildFlags": "-ldflags '-X main.Hello=World'"}) + }) + }) +} + func TestUnupportedCommandResponses(t *testing.T) { var got *dap.ErrorResponse runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { @@ -586,6 +608,10 @@ func TestBadLaunchRequests(t *testing.T) { expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), "Failed to launch: value '1' in 'args' attribute in debug configuration is not a string.") + client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "buildFlags": 123}) + expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), + "Failed to launch: 'buildFlags' attribute '123' in debug configuration is not a string.") + // Skip detailed message checks for potentially different OS-specific errors. client.LaunchRequest("exec", fixture.Path+"_does_not_exist", stopOnEntry) expectFailedToLaunch(client.ExpectErrorResponse(t)) @@ -596,6 +622,9 @@ func TestBadLaunchRequests(t *testing.T) { client.LaunchRequest("exec", fixture.Source, stopOnEntry) expectFailedToLaunch(client.ExpectErrorResponse(t)) // Not an executable + client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "buildFlags": "123"}) + expectFailedToLaunch(client.ExpectErrorResponse(t)) // Build error + // We failed to launch the program. Make sure shutdown still works. client.DisconnectRequest() dresp := client.ExpectDisconnectResponse(t)