From a9faeffebb11780fedb26cf64f4c8e74bd80ee39 Mon Sep 17 00:00:00 2001 From: brendandouglas Date: Thu, 14 Jun 2018 06:30:58 -0700 Subject: Skylark debugging protocol: remove conditional breakpoints, don't include frames in thread paused events. I started implementing conditional breakpoints server-side in unknown commit, but: - client-side implementations seem more common in debugging protocols - IntelliJ in particular has great support for client-side conditional breakpoints, but poor support for server-side - it's unnecessary extra complexity for the server -- simple line breakpoints + evaluation requests already give us conditional bps. Also removing frames from thread paused events. It results in poor performance for stepping and pausing (in which we get frame info for potentially dozens of threads), and again isn't usual for debuggers. Finally, ignore breakpoints when performing a client-requested evaluation. PiperOrigin-RevId: 200547972 --- .../lib/skylarkdebug/server/SkylarkDebugServerTest.java | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) (limited to 'src/test/java/com/google') diff --git a/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java b/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java index a0b4eefc43..f85a24ed8e 100644 --- a/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java @@ -171,13 +171,7 @@ public class SkylarkDebugServerTest { .setThreadPausedState( ThreadPausedState.newBuilder() .setPauseReason(PauseReason.ALL_THREADS_PAUSED) - .setLocation(expectedLocation) - .addFrame( - Frame.newBuilder() - .setFunctionName("") - .setLocation(expectedLocation) - .addScope(Scope.newBuilder().setName("global")) - .build())) + .setLocation(expectedLocation)) .build()); sendStartDebuggingRequest(); @@ -217,12 +211,7 @@ public class SkylarkDebugServerTest { .setThreadPausedState( ThreadPausedState.newBuilder() .setPauseReason(PauseReason.HIT_BREAKPOINT) - .setLocation(breakpoint.toBuilder().setColumnNumber(1)) - .addFrame( - Frame.newBuilder() - .setFunctionName("") - .setLocation(breakpoint.toBuilder().setColumnNumber(1)) - .addScope(Scope.newBuilder().setName("global")))) + .setLocation(breakpoint.toBuilder().setColumnNumber(1))) .build(); assertThat(client.unnumberedEvents) @@ -449,7 +438,6 @@ public class SkylarkDebugServerTest { ThreadPausedState pausedState = threads.getThread(0).getThreadPausedState(); assertThat(pausedState.getPauseReason()).isEqualTo(PauseReason.STEPPING); assertThat(pausedState.getLocation()).isEqualTo(expectedLocation); - assertThat(pausedState.getFrameCount()).isEqualTo(2); } @Test -- cgit v1.2.3