Skip to content

Commit

Permalink
service/dap: Add buildFlags error checking and tests in launch reques…
Browse files Browse the repository at this point in the history
…ts (#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 <polinasok@users.noreply.github.com>
  • Loading branch information
polinasok and polinasok authored May 28, 2020
1 parent 4a9b341 commit 4be75be
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 5 deletions.
14 changes: 14 additions & 0 deletions _fixtures/buildflagtest.go
Original file line number Diff line number Diff line change
@@ -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)
}
13 changes: 10 additions & 3 deletions service/dap/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
33 changes: 31 additions & 2 deletions service/dap/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{
Expand All @@ -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{}{
Expand All @@ -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() {
Expand All @@ -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) {
Expand Down Expand Up @@ -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))
Expand All @@ -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)
Expand Down

0 comments on commit 4be75be

Please sign in to comment.