diff options
Diffstat (limited to 'src')
8 files changed, 421 insertions, 153 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto b/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto index cc0de17909..a3df9e4291 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto +++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto @@ -35,6 +35,7 @@ message DebugRequest { ListFramesRequest list_frames = 104; StartDebuggingRequest start_debugging = 105; PauseThreadRequest pause_thread = 106; + GetChildrenRequest get_children = 107; } } @@ -98,6 +99,19 @@ message PauseThreadRequest { int64 thread_id = 1; } +// A request to list the children of a previously-communicated Value, such as +// its elements (for a list or dictionary), its fields (for a struct), and so +// forth. +message GetChildrenRequest { + // The identifier of the relevant thread. + int64 thread_id = 1; + + // The identifier of the value for which children are being requested. If the + // value has no children, an empty list will be returned in + // GetChildrenResponse. + int64 value_id = 2; +} + // There are two kinds of events: "responses", which correspond to a // DebugRequest sent by the client, and other asynchronous events that may be // sent by the server to notify the client of activity in the Skylark code being @@ -118,6 +132,7 @@ message DebugEvent { ListFramesResponse list_frames = 104; StartDebuggingResponse start_debugging = 105; PauseThreadResponse pause_thread = 106; + GetChildrenResponse get_children = 107; ThreadPausedEvent thread_paused = 1001; ThreadContinuedEvent thread_continued = 1002; @@ -162,6 +177,11 @@ message StartDebuggingResponse { message PauseThreadResponse { } +// The response to a GetChildrenRequest. +message GetChildrenResponse { + repeated Value children = 1; +} + // An event indicating that a thread was paused during execution. message ThreadPausedEvent { // The thread that was paused. @@ -307,7 +327,12 @@ message Value { // themselves do not have a meaningful type with respect to our rendering. string type = 3; - // Any child values associated with this value, such as its elements (for a - // list or dictionary), its fields (for a struct), and so forth. - repeated Value child = 4; + // Will be false if the value is known to have no children. May sometimes be + // true if this isn't yet known, in which case GetChildrenResponse#children + // will be empty. + bool has_children = 4; + + // An identifier for this value, used to request its children. The same value + // may be known by multiple ids. Not set for values without children. + int64 id = 5; } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebugEventHelper.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebugEventHelper.java index 648f9ab335..c0796fdc0b 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebugEventHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebugEventHelper.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Deb import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Error; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.EvaluateResponse; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Frame; +import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.GetChildrenResponse; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.ListFramesResponse; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.PauseThreadResponse; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.PausedThread; @@ -101,6 +102,13 @@ final class DebugEventHelper { .build(); } + static DebugEvent getChildrenResponse(long sequenceNumber, Collection<Value> children) { + return DebugEvent.newBuilder() + .setSequenceNumber(sequenceNumber) + .setGetChildren(GetChildrenResponse.newBuilder().addAllChildren(children)) + .build(); + } + static DebugEvent threadPausedEvent(PausedThread thread) { return DebugEvent.newBuilder() .setThreadPaused(ThreadPausedEvent.newBuilder().setThread(thread)) @@ -129,33 +137,36 @@ final class DebugEventHelper { .build(); } - static SkylarkDebuggingProtos.Frame getFrameProto(DebugFrame frame) { + static SkylarkDebuggingProtos.Frame getFrameProto(ThreadObjectMap objectMap, DebugFrame frame) { SkylarkDebuggingProtos.Frame.Builder builder = SkylarkDebuggingProtos.Frame.newBuilder() .setFunctionName(frame.functionName()) - .addAllScope(getScopes(frame)); + .addAllScope(getScopes(objectMap, frame)); if (frame.location() != null) { builder.setLocation(getLocationProto(frame.location())); } return builder.build(); } - private static ImmutableList<Scope> getScopes(DebugFrame frame) { + private static ImmutableList<Scope> getScopes(ThreadObjectMap objectMap, DebugFrame frame) { ImmutableMap<String, Object> localVars = frame.lexicalFrameBindings(); if (localVars.isEmpty()) { - return ImmutableList.of(getScope("global", frame.globalBindings())); + return ImmutableList.of(getScope(objectMap, "global", frame.globalBindings())); } Map<String, Object> globalVars = new LinkedHashMap<>(frame.globalBindings()); // remove shadowed bindings localVars.keySet().forEach(globalVars::remove); - return ImmutableList.of(getScope("local", localVars), getScope("global", globalVars)); + return ImmutableList.of( + getScope(objectMap, "local", localVars), getScope(objectMap, "global", globalVars)); } - private static SkylarkDebuggingProtos.Scope getScope(String name, Map<String, Object> bindings) { + private static SkylarkDebuggingProtos.Scope getScope( + ThreadObjectMap objectMap, String name, Map<String, Object> bindings) { SkylarkDebuggingProtos.Scope.Builder builder = SkylarkDebuggingProtos.Scope.newBuilder().setName(name); - bindings.forEach((s, o) -> builder.addBinding(DebuggerSerialization.getValueProto(s, o))); + bindings.forEach( + (s, o) -> builder.addBinding(DebuggerSerialization.getValueProto(objectMap, s, o))); return builder.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebuggerSerialization.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebuggerSerialization.java index e45a1fa294..0bc7b6fb01 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebuggerSerialization.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/DebuggerSerialization.java @@ -30,14 +30,16 @@ import java.util.Map; /** Helper class for creating {@link SkylarkDebuggingProtos.Value} from skylark objects. */ final class DebuggerSerialization { - static Value getValueProto(String label, Object value) { + static Value getValueProto(ThreadObjectMap objectMap, String label, Object value) { // TODO(bazel-team): prune cycles, and provide a way to limit breadth/depth of children reported + boolean hasChildren = hasChildren(value); return Value.newBuilder() .setLabel(label) // TODO(bazel-team): omit type details for non-Skylark values .setType(EvalUtils.getDataTypeName(value)) .setDescription(getDescription(value)) - .addAllChild(getChildren(value)) + .setHasChildren(hasChildren) + .setId(hasChildren ? objectMap.registerValue(value) : 0) .build(); } @@ -52,34 +54,63 @@ final class DebuggerSerialization { return Value.newBuilder().setLabel("Error").setDescription(errorMessage).build(); } - private static ImmutableList<Value> getChildren(Object value) { + private static boolean hasChildren(Object value) { + if (value instanceof ClassObject) { + // assuming ClassObject's have at least one child as a temporary optimization + // TODO(bazel-team): remove once child-listing logic is moved to SkylarkValue + return true; + } + if (value instanceof SkylarkNestedSet) { + return true; + } + if (value instanceof NestedSetView) { + return true; + } + if (value instanceof Map) { + return !((Map) value).isEmpty(); + } + if (value instanceof Map.Entry) { + return true; + } + if (value instanceof Iterable) { + return ((Iterable) value).iterator().hasNext(); + } + if (value.getClass().isArray()) { + return Array.getLength(value) > 0; + } + // fallback to assuming there are no children + return false; + } + + static ImmutableList<Value> getChildren(ThreadObjectMap objectMap, Object value) { // TODO(bazel-team): move child-listing logic to SkylarkValue where practical if (value instanceof ClassObject) { - return getChildren((ClassObject) value); + return getChildren(objectMap, (ClassObject) value); } if (value instanceof SkylarkNestedSet) { - return getChildren((SkylarkNestedSet) value); + return getChildren(objectMap, (SkylarkNestedSet) value); } if (value instanceof NestedSetView) { - return getChildren((NestedSetView) value); + return getChildren(objectMap, (NestedSetView) value); } if (value instanceof Map) { - return getChildren(((Map) value).entrySet()); + return getChildren(objectMap, ((Map) value).entrySet()); } if (value instanceof Map.Entry) { - return getChildren((Map.Entry) value); + return getChildren(objectMap, (Map.Entry) value); } if (value instanceof Iterable) { - return getChildren((Iterable) value); + return getChildren(objectMap, (Iterable) value); } if (value.getClass().isArray()) { - return getArrayChildren(value); + return getArrayChildren(objectMap, value); } // fallback to assuming there are no children return ImmutableList.of(); } - private static ImmutableList<Value> getChildren(ClassObject classObject) { + private static ImmutableList<Value> getChildren( + ThreadObjectMap objectMap, ClassObject classObject) { ImmutableList.Builder<Value> builder = ImmutableList.builder(); ImmutableList<String> keys; try { @@ -97,13 +128,14 @@ final class DebuggerSerialization { String.format("Error retrieving value for field '%s': %s", key, e.getMessage()))); } if (value != null) { - builder.add(getValueProto(key, value)); + builder.add(getValueProto(objectMap, key, value)); } } return builder.build(); } - private static ImmutableList<Value> getChildren(SkylarkNestedSet nestedSet) { + private static ImmutableList<Value> getChildren( + ThreadObjectMap objectMap, SkylarkNestedSet nestedSet) { Class<?> type = nestedSet.getContentType().getType(); return ImmutableList.<Value>builder() .add( @@ -112,35 +144,38 @@ final class DebuggerSerialization { .setType("Traversal order") .setDescription(nestedSet.getOrder().getSkylarkName()) .build()) - .addAll(getChildren(new NestedSetView<>(nestedSet.getSet(type)))) + .addAll(getChildren(objectMap, new NestedSetView<>(nestedSet.getSet(type)))) .build(); } - private static ImmutableList<Value> getChildren(NestedSetView<?> nestedSet) { + private static ImmutableList<Value> getChildren( + ThreadObjectMap objectMap, NestedSetView<?> nestedSet) { return ImmutableList.of( - getValueProto("directs", nestedSet.directs()), - getValueProto("transitives", nestedSet.transitives())); + getValueProto(objectMap, "directs", nestedSet.directs()), + getValueProto(objectMap, "transitives", nestedSet.transitives())); } - private static ImmutableList<Value> getChildren(Map.Entry<?, ?> entry) { + private static ImmutableList<Value> getChildren( + ThreadObjectMap objectMap, Map.Entry<?, ?> entry) { return ImmutableList.of( - getValueProto("key", entry.getKey()), getValueProto("value", entry.getValue())); + getValueProto(objectMap, "key", entry.getKey()), + getValueProto(objectMap, "value", entry.getValue())); } - private static ImmutableList<Value> getChildren(Iterable<?> iterable) { + private static ImmutableList<Value> getChildren(ThreadObjectMap objectMap, Iterable<?> iterable) { ImmutableList.Builder<Value> builder = ImmutableList.builder(); int index = 0; for (Object value : iterable) { - builder.add(getValueProto(String.format("[%d]", index++), value)); + builder.add(getValueProto(objectMap, String.format("[%d]", index++), value)); } return builder.build(); } - private static ImmutableList<Value> getArrayChildren(Object array) { + private static ImmutableList<Value> getArrayChildren(ThreadObjectMap objectMap, Object array) { ImmutableList.Builder<Value> builder = ImmutableList.builder(); int index = 0; for (int i = 0; i < Array.getLength(array); i++) { - builder.add(getValueProto(String.format("[%d]", index++), Array.get(array, i))); + builder.add(getValueProto(objectMap, String.format("[%d]", index++), Array.get(array, i))); } return builder.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java index 6e605895ef..d87bfdadd3 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java @@ -177,6 +177,8 @@ public final class SkylarkDebugServer implements DebugServer { return pauseThread(sequenceNumber, request.getPauseThread()); case EVALUATE: return evaluate(sequenceNumber, request.getEvaluate()); + case GET_CHILDREN: + return getChildren(sequenceNumber, request.getGetChildren()); case PAYLOAD_NOT_SET: DebugEventHelper.error(sequenceNumber, "No request payload found"); } @@ -210,6 +212,15 @@ public final class SkylarkDebugServer implements DebugServer { sequenceNumber, threadHandler.evaluate(request.getThreadId(), request.getStatement())); } + /** Handles a {@code GetChildrenRequest} and returns its response. */ + private SkylarkDebuggingProtos.DebugEvent getChildren( + long sequenceNumber, SkylarkDebuggingProtos.GetChildrenRequest request) + throws DebugRequestException { + return DebugEventHelper.getChildrenResponse( + sequenceNumber, + threadHandler.getChildrenForValue(request.getThreadId(), request.getValueId())); + } + /** Handles a {@code ContinueExecutionRequest} and returns its response. */ private SkylarkDebuggingProtos.DebugEvent continueExecution( long sequenceNumber, SkylarkDebuggingProtos.ContinueExecutionRequest request) diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java index b390b613a2..b2828f8277 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Breakpoint; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Error; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.PauseReason; +import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Value; import com.google.devtools.build.lib.syntax.Debuggable; import com.google.devtools.build.lib.syntax.Debuggable.ReadyToPause; import com.google.devtools.build.lib.syntax.Environment; @@ -50,12 +51,15 @@ final class ThreadHandler { /** Used to block execution of threads */ final Semaphore semaphore; + final ThreadObjectMap objectMap; + PausedThreadState(long id, String name, Debuggable debuggable, Location location) { this.id = id; this.name = name; this.debuggable = debuggable; this.location = location; this.semaphore = new Semaphore(0); + this.objectMap = new ThreadObjectMap(); } } @@ -230,14 +234,33 @@ final class ThreadHandler { .debuggable .listFrames(thread.location) .stream() - .map(DebugEventHelper::getFrameProto) + .map(frame -> DebugEventHelper.getFrameProto(thread.objectMap, frame)) .collect(toImmutableList()); } } + ImmutableList<Value> getChildrenForValue(long threadId, long valueId) + throws DebugRequestException { + ThreadObjectMap objectMap; + synchronized (this) { + PausedThreadState thread = pausedThreads.get(threadId); + if (thread == null) { + throw new DebugRequestException( + String.format("Thread %s is not paused or does not exist.", threadId)); + } + objectMap = thread.objectMap; + } + Object value = objectMap.getValue(valueId); + if (value == null) { + throw new DebugRequestException("Couldn't retrieve children; object not found."); + } + return DebuggerSerialization.getChildren(objectMap, value); + } + SkylarkDebuggingProtos.Value evaluate(long threadId, String statement) throws DebugRequestException { Debuggable debuggable; + ThreadObjectMap objectMap; synchronized (this) { PausedThreadState thread = pausedThreads.get(threadId); if (thread == null) { @@ -245,13 +268,15 @@ final class ThreadHandler { String.format("Thread %s is not paused or does not exist.", threadId)); } debuggable = thread.debuggable; + objectMap = thread.objectMap; } - // no need to evaluate within the synchronize block: for paused threads, debuggable is only - // accessed in response to a client request, and requests are handled serially + // no need to evaluate within the synchronize block: for paused threads, the debuggable and + // object map are only accessed in response to a client request, and requests are handled + // serially // TODO(bazel-team): support asynchronous replies, and use finer-grained locks try { Object result = doEvaluate(debuggable, statement); - return DebuggerSerialization.getValueProto("Evaluation result", result); + return DebuggerSerialization.getValueProto(objectMap, "Evaluation result", result); } catch (EvalException | InterruptedException e) { throw new DebugRequestException(e.getMessage()); } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadObjectMap.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadObjectMap.java new file mode 100644 index 0000000000..4e55c6105e --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadObjectMap.java @@ -0,0 +1,54 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.skylarkdebug.server; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; +import javax.annotation.Nullable; + +/** + * A map of identifier to value for all the values with children which have been communicated to the + * debug client for a particular, currently-paused thread. + * + * <p>Objects without children (as defined by the debugging protocol) should not be stored here. + * + * <p>Object identifiers apply only to a single thread, and aren't relevant to other threads. + * + * <p>This state is retained only while the thread is paused. + */ +final class ThreadObjectMap { + private final AtomicLong lastId = new AtomicLong(1); + private final Map<Long, Object> objectMap = new ConcurrentHashMap<>(); + + /** + * Assigns and returns a unique identifier for this object, and stores a reference in the + * identifier to object map. + * + * <p>If called multiple times for a given object, a different identifier will be returned each + * time. The object can be retrieved by all such identifiers. + */ + long registerValue(Object value) { + long id = lastId.getAndIncrement(); + objectMap.put(id, value); + return id; + } + + /** Returns the object corresponding to the given identifier, if one exists. */ + @Nullable + Object getValue(long identifier) { + return objectMap.get(identifier); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/DebuggerSerializationTest.java b/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/DebuggerSerializationTest.java index 525f1ce005..52b208f076 100644 --- a/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/DebuggerSerializationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/DebuggerSerializationTest.java @@ -26,8 +26,10 @@ import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Val import com.google.devtools.build.lib.syntax.EvalUtils; import com.google.devtools.build.lib.syntax.Printer; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -36,27 +38,47 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public final class DebuggerSerializationTest { + private final ThreadObjectMap dummyObjectMap = new ThreadObjectMap(); + + /** + * Returns the {@link Value} proto message corresponding to the given object and label. Subsequent + * calls may return values with different IDs. + */ + private Value getValueProto(String label, Object value) { + return DebuggerSerialization.getValueProto(dummyObjectMap, label, value); + } + + private ImmutableList<Value> getChildren(Value value) { + Object object = dummyObjectMap.getValue(value.getId()); + return object != null + ? DebuggerSerialization.getChildren(dummyObjectMap, object) + : ImmutableList.of(); + } + @Test public void testSimpleNestedSet() { Set<String> children = ImmutableSet.of("a", "b"); SkylarkNestedSet set = SkylarkNestedSet.of(Object.class, NestedSetBuilder.stableOrder().addAll(children).build()); - Value value = DebuggerSerialization.getValueProto("name", set); + Value value = getValueProto("name", set); assertTypeAndDescription(set, value); - assertThat(value.getChildList()).hasSize(3); - assertThat(value.getChild(0)) + assertThat(value.getHasChildren()).isTrue(); + assertThat(value.getLabel()).isEqualTo("name"); + + List<Value> childValues = getChildren(value); + + assertThat(childValues.get(0)) .isEqualTo( Value.newBuilder() .setLabel("order") .setType("Traversal order") .setDescription("default") .build()); - assertEqualIgnoringTypeAndDescription( - value.getChild(1), DebuggerSerialization.getValueProto("directs", children)); - assertEqualIgnoringTypeAndDescription( - value.getChild(2), DebuggerSerialization.getValueProto("transitives", ImmutableList.of())); + assertEqualIgnoringTypeDescriptionAndId(childValues.get(1), getValueProto("directs", children)); + assertEqualIgnoringTypeDescriptionAndId( + childValues.get(2), getValueProto("transitives", ImmutableList.of())); } @Test @@ -72,45 +94,40 @@ public final class DebuggerSerializationTest { .addTransitive(innerNestedSet) .build()); - Value value = DebuggerSerialization.getValueProto("name", outerSet); + Value value = getValueProto("name", outerSet); + List<Value> childValues = getChildren(value); assertTypeAndDescription(outerSet, value); - assertThat(value.getChildList()).hasSize(3); - assertThat(value.getChild(0)) + assertThat(childValues).hasSize(3); + assertThat(childValues.get(0)) .isEqualTo( Value.newBuilder() .setLabel("order") .setType("Traversal order") .setDescription("topological") .build()); - assertEqualIgnoringTypeAndDescription( - value.getChild(1), DebuggerSerialization.getValueProto("directs", directChildren)); - assertEqualIgnoringTypeAndDescription( - value.getChild(2), - DebuggerSerialization.getValueProto( - "transitives", ImmutableList.of(new NestedSetView<>(innerNestedSet)))); + assertEqualIgnoringTypeDescriptionAndId( + childValues.get(1), getValueProto("directs", directChildren)); + assertEqualIgnoringTypeDescriptionAndId( + childValues.get(2), + getValueProto("transitives", ImmutableList.of(new NestedSetView<>(innerNestedSet)))); } @Test public void testSimpleMap() { Map<String, Integer> map = ImmutableMap.of("a", 1, "b", 2); - Value value = DebuggerSerialization.getValueProto("name", map); + Value value = getValueProto("name", map); + List<Value> childValues = getChildren(value); assertTypeAndDescription(map, value); - assertThat(value.getChildList()).hasSize(2); - assertThat(value.getChild(0).getLabel()).isEqualTo("[0]"); - assertThat(value.getChild(0).getChildList()) - .isEqualTo( - ImmutableList.of( - DebuggerSerialization.getValueProto("key", "a"), - DebuggerSerialization.getValueProto("value", 1))); - assertThat(value.getChild(1).getLabel()).isEqualTo("[1]"); - assertThat(value.getChild(1).getChildList()) - .isEqualTo( - ImmutableList.of( - DebuggerSerialization.getValueProto("key", "b"), - DebuggerSerialization.getValueProto("value", 2))); + assertThat(childValues).hasSize(2); + assertThat(childValues.get(0).getLabel()).isEqualTo("[0]"); + assertThat(getChildren(childValues.get(0))) + .isEqualTo(ImmutableList.of(getValueProto("key", "a"), getValueProto("value", 1))); + assertThat(childValues.get(1).getLabel()).isEqualTo("[1]"); + assertThat(getChildren(childValues.get(1))) + .isEqualTo(ImmutableList.of(getValueProto("key", "b"), getValueProto("value", 2))); } @Test @@ -118,72 +135,73 @@ public final class DebuggerSerializationTest { Set<String> set = ImmutableSet.of("a", "b"); Map<String, Object> map = ImmutableMap.of("a", set); - Value value = DebuggerSerialization.getValueProto("name", map); + Value value = getValueProto("name", map); + List<Value> childValues = getChildren(value); assertTypeAndDescription(map, value); - assertThat(value.getChildList()).hasSize(1); - assertThat(value.getChild(0).getLabel()).isEqualTo("[0]"); - assertThat(value.getChild(0).getChildList()) + assertThat(childValues).hasSize(1); + assertThat(childValues.get(0).getLabel()).isEqualTo("[0]"); + assertThat(clearIds(getChildren(childValues.get(0)))) .isEqualTo( - ImmutableList.of( - DebuggerSerialization.getValueProto("key", "a"), - DebuggerSerialization.getValueProto("value", set))); + ImmutableList.of(getValueProto("key", "a"), clearId(getValueProto("value", set)))); } @Test public void testSimpleIterable() { Iterable<Integer> iter = ImmutableList.of(1, 2); - Value value = DebuggerSerialization.getValueProto("name", iter); + Value value = getValueProto("name", iter); + List<Value> childValues = getChildren(value); assertTypeAndDescription(iter, value); - assertThat(value.getChildList()).hasSize(2); - assertThat(value.getChild(0)).isEqualTo(DebuggerSerialization.getValueProto("[0]", 1)); - assertThat(value.getChild(1)).isEqualTo(DebuggerSerialization.getValueProto("[1]", 2)); + assertThat(childValues).hasSize(2); + assertThat(childValues.get(0)).isEqualTo(getValueProto("[0]", 1)); + assertThat(childValues.get(1)).isEqualTo(getValueProto("[1]", 2)); } @Test public void testNestedIterable() { Iterable<Object> iter = ImmutableList.of(ImmutableList.of(1, 2)); - Value value = DebuggerSerialization.getValueProto("name", iter); + Value value = getValueProto("name", iter); + List<Value> childValues = getChildren(value); assertTypeAndDescription(iter, value); - assertThat(value.getChildList()).hasSize(1); - assertThat(value.getChild(0)) - .isEqualTo(DebuggerSerialization.getValueProto("[0]", ImmutableList.of(1, 2))); + assertThat(childValues).hasSize(1); + assertValuesEqualIgnoringId(childValues.get(0), getValueProto("[0]", ImmutableList.of(1, 2))); } @Test public void testSimpleArray() { int[] array = new int[] {1, 2}; - Value value = DebuggerSerialization.getValueProto("name", array); + Value value = getValueProto("name", array); + List<Value> childValues = getChildren(value); assertTypeAndDescription(array, value); - assertThat(value.getChildList()).hasSize(2); - assertThat(value.getChild(0)).isEqualTo(DebuggerSerialization.getValueProto("[0]", 1)); - assertThat(value.getChild(1)).isEqualTo(DebuggerSerialization.getValueProto("[1]", 2)); + assertThat(childValues).hasSize(2); + assertThat(childValues.get(0)).isEqualTo(getValueProto("[0]", 1)); + assertThat(childValues.get(1)).isEqualTo(getValueProto("[1]", 2)); } @Test public void testNestedArray() { Object[] array = new Object[] {1, ImmutableList.of(2, 3)}; - Value value = DebuggerSerialization.getValueProto("name", array); + Value value = getValueProto("name", array); + List<Value> childValues = getChildren(value); assertTypeAndDescription(array, value); - assertThat(value.getChildList()).hasSize(2); - assertThat(value.getChild(0)).isEqualTo(DebuggerSerialization.getValueProto("[0]", 1)); - assertThat(value.getChild(1)) - .isEqualTo(DebuggerSerialization.getValueProto("[1]", ImmutableList.of(2, 3))); + assertThat(childValues).hasSize(2); + assertThat(childValues.get(0)).isEqualTo(getValueProto("[0]", 1)); + assertValuesEqualIgnoringId(childValues.get(1), getValueProto("[1]", ImmutableList.of(2, 3))); } @Test public void testUnrecognizedObjectOrSkylarkPrimitiveHasNoChildren() { - assertThat(DebuggerSerialization.getValueProto("name", 1).getChildList()).isEmpty(); - assertThat(DebuggerSerialization.getValueProto("name", "string").getChildList()).isEmpty(); - assertThat(DebuggerSerialization.getValueProto("name", new Object()).getChildList()).isEmpty(); + assertThat(getValueProto("name", 1).getHasChildren()).isFalse(); + assertThat(getValueProto("name", "string").getHasChildren()).isFalse(); + assertThat(getValueProto("name", new Object()).getHasChildren()).isFalse(); } private static void assertTypeAndDescription(Object object, Value value) { @@ -192,14 +210,30 @@ public final class DebuggerSerializationTest { } /** - * Type and description are implementation dependent (e.g. NestedSetView#directs returns a list - * instead of a set if there are no duplicates, which changes both 'type' and 'description'). + * Type, description, and ID are implementation dependent (e.g. NestedSetView#directs returns a + * list instead of a set if there are no duplicates, which changes both 'type' and 'description'). */ - private static void assertEqualIgnoringTypeAndDescription(Value value1, Value value2) { + private void assertEqualIgnoringTypeDescriptionAndId(Value value1, Value value2) { assertThat(value1.getLabel()).isEqualTo(value2.getLabel()); - assertThat(value1.getChildCount()).isEqualTo(value2.getChildCount()); - for (int i = 0; i < value1.getChildCount(); i++) { - assertEqualIgnoringTypeAndDescription(value1.getChild(i), value2.getChild(i)); + + List<Value> children1 = getChildren(value1); + List<Value> children2 = getChildren(value2); + + assertThat(children1).hasSize(children2.size()); + for (int i = 0; i < children1.size(); i++) { + assertEqualIgnoringTypeDescriptionAndId(children1.get(i), children2.get(i)); } } + + private void assertValuesEqualIgnoringId(Value value1, Value value2) { + assertThat(clearId(value1)).isEqualTo(clearId(value2)); + } + + private Value clearId(Value value) { + return value.toBuilder().clearId().build(); + } + + private List<Value> clearIds(List<Value> values) { + return values.stream().map(this::clearId).collect(Collectors.toList()); + } } 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 5a7c12273c..b1cf3d336a 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 @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Set import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.StartDebuggingRequest; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.StartDebuggingResponse; import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Stepping; +import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos.Value; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.DebugServerUtils; import com.google.devtools.build.lib.syntax.Environment; @@ -51,6 +52,7 @@ import java.net.InetAddress; import java.net.ServerSocket; import java.time.Duration; import java.util.Collection; +import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -70,10 +72,26 @@ public class SkylarkDebugServerTest { private final Scratch scratch = new Scratch(); private final EventCollectionApparatus events = new EventCollectionApparatus(EventKind.ALL_EVENTS); + private final ThreadObjectMap dummyObjectMap = new ThreadObjectMap(); private MockDebugClient client; private SkylarkDebugServer server; + /** + * Returns the {@link Value} proto message corresponding to the given object and label. Subsequent + * calls may return values with different IDs. + */ + private Value getValueProto(String label, Object value) { + return DebuggerSerialization.getValueProto(dummyObjectMap, label, value); + } + + private ImmutableList<Value> getChildren(Value value) { + Object object = dummyObjectMap.getValue(value.getId()); + return object != null + ? DebuggerSerialization.getChildren(dummyObjectMap, object) + : ImmutableList.of(); + } + @Before public void setUpServerAndClient() throws Exception { ServerSocket serverSocket = new ServerSocket(0, 1, InetAddress.getByName(null)); @@ -338,18 +356,47 @@ public class SkylarkDebugServerTest { ListFramesResponse frames = listFrames(threadId); assertThat(frames.getFrameCount()).isEqualTo(1); - assertThat(frames.getFrame(0)) + assertFramesEqualIgnoringValueIdentifiers( + frames.getFrame(0), + Frame.newBuilder() + .setFunctionName("<top level>") + .setLocation(breakpoint.toBuilder().setColumnNumber(1)) + .addScope( + Scope.newBuilder() + .setName("global") + .addBinding( + getValueProto("x", SkylarkList.createImmutable(ImmutableList.of(1, 2, 3))))) + .build()); + } + + @Test + public void testGetChildrenRequest() throws Exception { + sendStartDebuggingRequest(); + BuildFileAST buildFile = parseBuildFile("/a/build/file/BUILD", "x = [1,2,3]", "y = [2,3,4]"); + Environment env = newEnvironment(); + + Location breakpoint = + Location.newBuilder().setLineNumber(2).setPath("/a/build/file/BUILD").build(); + setBreakpoints(ImmutableList.of(breakpoint)); + + Thread evaluationThread = execInWorkerThread(buildFile, env); + long threadId = evaluationThread.getId(); + + // wait for breakpoint to be hit + client.waitForEvent(DebugEvent::hasThreadPaused, Duration.ofSeconds(5)); + + ListFramesResponse frames = listFrames(threadId); + Value xValue = frames.getFrame(0).getScope(0).getBinding(0); + + assertValuesEqualIgnoringId( + xValue, getValueProto("x", SkylarkList.createImmutable(ImmutableList.of(1, 2, 3)))); + + List<Value> children = getChildren(xValue); + + assertThat(children) .isEqualTo( - Frame.newBuilder() - .setFunctionName("<top level>") - .setLocation(breakpoint.toBuilder().setColumnNumber(1)) - .addScope( - Scope.newBuilder() - .setName("global") - .addBinding( - DebuggerSerialization.getValueProto( - "x", SkylarkList.createImmutable(ImmutableList.of(1, 2, 3))))) - .build()); + ImmutableList.of( + getValueProto("[0]", 1), getValueProto("[1]", 2), getValueProto("[2]", 3))); } @Test @@ -380,39 +427,39 @@ public class SkylarkDebugServerTest { ListFramesResponse frames = listFrames(threadId); assertThat(frames.getFrameCount()).isEqualTo(2); - assertThat(frames.getFrame(0)) - .isEqualTo( - Frame.newBuilder() - .setFunctionName("fn") - .setLocation(breakpoint.toBuilder().setColumnNumber(3)) - .addScope( - Scope.newBuilder() - .setName("local") - .addBinding(DebuggerSerialization.getValueProto("a", 2)) - .addBinding(DebuggerSerialization.getValueProto("b", 1))) - .addScope( - Scope.newBuilder() - .setName("global") - .addBinding(DebuggerSerialization.getValueProto("c", 3)) - .addBinding(DebuggerSerialization.getValueProto("fn", env.lookup("fn")))) - .build()); + assertFramesEqualIgnoringValueIdentifiers( + frames.getFrame(0), + Frame.newBuilder() + .setFunctionName("fn") + .setLocation(breakpoint.toBuilder().setColumnNumber(3)) + .addScope( + Scope.newBuilder() + .setName("local") + .addBinding(getValueProto("a", 2)) + .addBinding(getValueProto("b", 1))) + .addScope( + Scope.newBuilder() + .setName("global") + .addBinding(getValueProto("c", 3)) + .addBinding(getValueProto("fn", env.lookup("fn")))) + .build()); - assertThat(frames.getFrame(1)) - .isEqualTo( - Frame.newBuilder() - .setFunctionName("<top level>") - .setLocation( - Location.newBuilder() - .setPath("/a/build/file/test.bzl") - .setLineNumber(7) - .setColumnNumber(1)) - .addScope( - Scope.newBuilder() - .setName("global") - .addBinding(DebuggerSerialization.getValueProto("a", 1)) - .addBinding(DebuggerSerialization.getValueProto("c", 3)) - .addBinding(DebuggerSerialization.getValueProto("fn", env.lookup("fn")))) - .build()); + assertFramesEqualIgnoringValueIdentifiers( + frames.getFrame(1), + Frame.newBuilder() + .setFunctionName("<top level>") + .setLocation( + Location.newBuilder() + .setPath("/a/build/file/test.bzl") + .setLineNumber(7) + .setColumnNumber(1)) + .addScope( + Scope.newBuilder() + .setName("global") + .addBinding(getValueProto("a", 1)) + .addBinding(getValueProto("c", 3)) + .addBinding(getValueProto("fn", env.lookup("fn")))) + .build()); } @Test @@ -439,8 +486,7 @@ public class SkylarkDebugServerTest { EvaluateRequest.newBuilder().setThreadId(threadId).setStatement("x[1]").build()) .build()); assertThat(response.hasEvaluate()).isTrue(); - assertThat(response.getEvaluate().getResult()) - .isEqualTo(DebuggerSerialization.getValueProto("Evaluation result", 2)); + assertThat(response.getEvaluate().getResult()).isEqualTo(getValueProto("Evaluation result", 2)); } @Test @@ -471,14 +517,12 @@ public class SkylarkDebugServerTest { .build()); assertThat(response.getEvaluate().getResult()) .isEqualTo( - DebuggerSerialization.getValueProto( + getValueProto( "Evaluation result", SkylarkList.createImmutable(ImmutableList.of(5, 6)))); ListFramesResponse frames = listFrames(threadId); assertThat(frames.getFrame(0).getScope(0).getBindingList()) - .contains( - DebuggerSerialization.getValueProto( - "x", SkylarkList.createImmutable(ImmutableList.of(5, 6)))); + .contains(getValueProto("x", SkylarkList.createImmutable(ImmutableList.of(5, 6)))); } @Test @@ -508,13 +552,11 @@ public class SkylarkDebugServerTest { .build()) .build()); assertThat(response.getEvaluate().getResult()) - .isEqualTo(DebuggerSerialization.getValueProto("Evaluation result", Runtime.NONE)); + .isEqualTo(getValueProto("Evaluation result", Runtime.NONE)); ListFramesResponse frames = listFrames(threadId); assertThat(frames.getFrame(0).getScope(0).getBindingList()) - .contains( - DebuggerSerialization.getValueProto( - "x", SkylarkList.createImmutable(ImmutableList.of(1, 2, 3, 4)))); + .contains(getValueProto("x", SkylarkList.createImmutable(ImmutableList.of(1, 2, 3, 4)))); } @Test @@ -754,4 +796,35 @@ public class SkylarkDebugServerTest { thread.start(); return thread; } + + /** + * Asserts that the given frames are equal after clearing the identifier from all {@link Value}s. + */ + private void assertFramesEqualIgnoringValueIdentifiers(Frame frame1, Frame frame2) { + assertThat(clearIds(frame1)).isEqualTo(clearIds(frame2)); + } + + private static Frame clearIds(Frame frame) { + Frame.Builder builder = frame.toBuilder(); + for (int i = 0; i < frame.getScopeCount(); i++) { + builder.setScope(i, clearIds(builder.getScope(i))); + } + return builder.build(); + } + + private static Scope clearIds(Scope scope) { + Scope.Builder builder = scope.toBuilder(); + for (int i = 0; i < scope.getBindingCount(); i++) { + builder.getBindingBuilder(i).clearId(); + } + return builder.build(); + } + + private void assertValuesEqualIgnoringId(Value value1, Value value2) { + assertThat(clearId(value1)).isEqualTo(clearId(value2)); + } + + private static Value clearId(Value value) { + return value.toBuilder().clearId().build(); + } } |