service/dap: clarify default attributes behavior with tests (#2365)

* service/dap: clarify default attributes behavior with tests

* Minor clean-up post-merge

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>

service/dap/server_test.go

@@ -2264,7 +2264,6 @@ func TestLaunchDebugRequest(t *testing.T) {
 		// 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, "launch", func() {
-			// Use the default output directory.
 			client.LaunchRequestWithArgs(map[string]interface{}{
 				"mode": "debug", "program": fixture.Source, "output": "__mydir"})
 		}, fixture.Source)
@@ -2289,6 +2288,31 @@ func TestLaunchDebugRequest(t *testing.T) {
 	}
 }
 
+// TestLaunchRequestDefaults tests defaults for launch attribute that are explicit in other tests.
+func TestLaunchRequestDefaults(t *testing.T) {
+	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSession(t, client, "launch", func() {
+			client.LaunchRequestWithArgs(map[string]interface{}{
+				"mode": "" /*"debug" by default*/, "program": fixture.Source, "output": "__mydir"})
+		}, fixture.Source)
+	})
+	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSession(t, client, "launch", func() {
+			// Use the default output directory.
+			client.LaunchRequestWithArgs(map[string]interface{}{
+				/*"mode":"debug" by default*/ "program": fixture.Source, "output": "__mydir"})
+		}, fixture.Source)
+	})
+	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSession(t, client, "launch", func() {
+			// Use the default output directory.
+			client.LaunchRequestWithArgs(map[string]interface{}{
+				"mode": "debug", "program": fixture.Source})
+			// writes to default output dir __debug_bin
+		}, fixture.Source)
+	})
+}
+
 func TestLaunchTestRequest(t *testing.T) {
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
 		runDebugSession(t, client, "launch", func() {
@@ -2326,9 +2350,8 @@ func TestLaunchRequestWithBuildFlags(t *testing.T) {
 			// 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,
+				"mode": "debug", "program": fixture.Source, "output": "__mydir",
 				"buildFlags": "-ldflags '-X main.Hello=World'"})
-			// will write to default output dir __debug_bin
 		}, fixture.Source)
 	})
 }
@@ -2353,7 +2376,8 @@ func TestAttachRequest(t *testing.T) {
 		runDebugSessionWithBPs(t, client, "attach",
 			// Attach
 			func() {
-				client.AttachRequest(map[string]interface{}{"mode": "local", "processId": cmd.Process.Pid, "stopOnEntry": false})
+				client.AttachRequest(map[string]interface{}{
+					/*"mode": "local" by default*/ "processId": cmd.Process.Pid, "stopOnEntry": false})
 			},
 			// Set breakpoints
 			fixture.Source, []int{8},
@@ -2521,18 +2545,20 @@ func TestBadLaunchRequests(t *testing.T) {
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: The program attribute is missing in debug configuration.")
 
-		client.LaunchRequestWithArgs(map[string]interface{}{"program": 12345})
+		// Bad "program"
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": 12345})
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: The program attribute is missing in debug configuration.")
 
-		client.LaunchRequestWithArgs(map[string]interface{}{"program": nil})
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": nil})
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: The program attribute is missing in debug configuration.")
 
-		client.LaunchRequestWithArgs(map[string]interface{}{})
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug"})
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: The program attribute is missing in debug configuration.")
 
+		// Bad "mode"
 		client.LaunchRequest("remote", fixture.Path, stopOnEntry)
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: Unsupported 'mode' value \"remote\" in debug configuration.")
@@ -2545,6 +2571,15 @@ func TestBadLaunchRequests(t *testing.T) {
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: Unsupported 'mode' value %!q(float64=12345) in debug configuration.")
 
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": ""}) // empty mode defaults to "debug" (not an error)
+		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
+			"Failed to launch: The program attribute is missing in debug configuration.")
+
+		client.LaunchRequestWithArgs(map[string]interface{}{}) // missing mode defaults to "debug" (not an error)
+		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
+			"Failed to launch: The program attribute is missing in debug configuration.")
+
+		// Bad "args"
 		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "exec", "program": fixture.Path, "args": nil})
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: 'args' attribute '<nil>' in debug configuration is not an array.")
@@ -2557,26 +2592,26 @@ func TestBadLaunchRequests(t *testing.T) {
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: value '1' in 'args' attribute in debug configuration is not a string.")
 
+		// Bad "buildFlags"
 		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))
+		expectFailedToLaunch(client.ExpectErrorResponse(t)) // No such file or directory
 
 		client.LaunchRequest("debug", fixture.Path+"_does_not_exist", stopOnEntry)
-		expectFailedToLaunch(client.ExpectErrorResponse(t)) // Build error
+		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), "Failed to launch: Build error: exit status 1")
+
+		client.LaunchRequest("" /*debug by default*/, fixture.Path+"_does_not_exist", stopOnEntry)
+		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), "Failed to launch: Build error: exit status 1")
 
 		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"})
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "buildFlags": "bad flags"})
-		expectFailedToLaunch(client.ExpectErrorResponse(t)) // Build error
+		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), "Failed to launch: Build error: exit status 1")
-
-		client.LaunchRequest("", fixture.Path, stopOnEntry)
-		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
-			"Failed to launch: Build error: exit status 1")
 
 		// We failed to launch the program. Make sure shutdown still works.
 		client.DisconnectRequest()
@@ -2615,6 +2650,7 @@ func TestBadAttachRequest(t *testing.T) {
 			}
 		}
 
+		// Bad "mode"
 		client.AttachRequest(map[string]interface{}{"mode": "remote"})
 		expectFailedToAttachWithMessage(client.ExpectErrorResponse(t),
 			"Failed to attach: Unsupported 'mode' value \"remote\" in debug configuration")
@@ -2627,28 +2663,37 @@ func TestBadAttachRequest(t *testing.T) {
 		expectFailedToAttachWithMessage(client.ExpectErrorResponse(t),
 			"Failed to attach: Unsupported 'mode' value %!q(float64=123) in debug configuration")
 
-		client.AttachRequest(map[string]interface{}{"mode": ""}) // defaults to "local"
+		client.AttachRequest(map[string]interface{}{"mode": ""}) // empty mode defaults to "local" (not an error)
 		expectFailedToAttachWithMessage(client.ExpectErrorResponse(t),
 			"Failed to attach: The 'processId' attribute is missing in debug configuration")
 
-		client.AttachRequest(map[string]interface{}{}) // defaults to "local"
+		client.AttachRequest(map[string]interface{}{}) // no mode defaults to "local" (not an error)
 		expectFailedToAttachWithMessage(client.ExpectErrorResponse(t),
 			"Failed to attach: The 'processId' attribute is missing in debug configuration")
 
-		client.AttachRequest(map[string]interface{}{"processId": 0}) // defaults to "local"
+		// Bad "processId"
+		client.AttachRequest(map[string]interface{}{"mode": "local"})
 		expectFailedToAttachWithMessage(client.ExpectErrorResponse(t),
 			"Failed to attach: The 'processId' attribute is missing in debug configuration")
 
-		client.AttachRequest(map[string]interface{}{"processId": "1"}) // defaults to "local"
+		client.AttachRequest(map[string]interface{}{"mode": "local", "processId": nil})
 		expectFailedToAttachWithMessage(client.ExpectErrorResponse(t),
 			"Failed to attach: The 'processId' attribute is missing in debug configuration")
 
-		client.AttachRequest(map[string]interface{}{"processId": 1}) // defaults to "local"
+		client.AttachRequest(map[string]interface{}{"mode": "local", "processId": 0})
+		expectFailedToAttachWithMessage(client.ExpectErrorResponse(t),
+			"Failed to attach: The 'processId' attribute is missing in debug configuration")
+
+		client.AttachRequest(map[string]interface{}{"mode": "local", "processId": "1"})
+		expectFailedToAttachWithMessage(client.ExpectErrorResponse(t),
+			"Failed to attach: The 'processId' attribute is missing in debug configuration")
+
+		client.AttachRequest(map[string]interface{}{"mode": "local", "processId": 1})
 		// The exact message varies on different systems, so skip that check
 		expectFailedToAttach(client.ExpectErrorResponse(t)) // could not attach to pid 1
 
 		// This will make debugger.(*Debugger) panic, which we will catch as an internal error.
-		client.AttachRequest(map[string]interface{}{"processId": -1}) // defaults to "local"
+		client.AttachRequest(map[string]interface{}{"mode": "local", "processId": -1})
 		er := client.ExpectErrorResponse(t)
 		if er.RequestSeq != seqCnt {
 			t.Errorf("RequestSeq got %d, want %d", seqCnt, er.RequestSeq)