[DebuggerV2] Highlight stack frame being shown in SourceCodeComponent (…

…#3550)

* Motivation for features / changes
  * Make it clearer in the StackFrameComponent which frame (if any) is being highlighted in the SourceCodeComponent.
* Technical description of changes
  * In StackFrameContainer's internal selector, add a boolean field to the return value called `focused`. It is used in the template ng html to apply a CSS class (`.focused-stack-frame`) to the focused stack frame and only the focused stack frame.
  * While adding new unit test for the new feature, simplify existing ones by using `overrideSelector`.
  * To conform to style guide, replace `toEqual()` with `toBe()` where applicable in the touched unit tests.
* Screenshots of UI changes
  * ![image](https://user-images.githubusercontent.com/16824702/80288736-ed102300-8707-11ea-9638-7aa197a23ff6.png)
* Detailed steps to verify changes work correctly (as executed by you)
  * Unit test added
  * Existing unit tests for `StackFrameContainer` are simplified by replacing `setState()` with `overrideSelector()`.

tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/debugger_container_test.ts

@@ -37,6 +37,10 @@ import {
   AlertType,
   TensorDebugMode,
 } from './store/debugger_types';
+import {
+  getFocusedExecutionStackFrames,
+  getFocusedSourceLineSpec,
+} from './store';
 import {
   createAlertsState,
   createDebuggerState,
@@ -559,184 +563,125 @@ describe('Debugger Container', () => {
   });
 
   describe('Stack Trace module', () => {
-    it('Shows non-empty stack frames correctly', () => {
+    it('Shows non-empty stack frames; highlights focused frame', () => {
       const fixture = TestBed.createComponent(StackTraceContainer);
-      fixture.detectChanges();
-
       const stackFrame0 = createTestStackFrame();
       const stackFrame1 = createTestStackFrame();
       const stackFrame2 = createTestStackFrame();
-      store.setState(
-        createState(
-          createDebuggerState({
-            executions: {
-              numExecutionsLoaded: {
-                state: DataLoadState.LOADED,
-                lastLoadedTimeInMs: 111,
-              },
-              executionDigestsLoaded: {
-                state: DataLoadState.LOADED,
-                lastLoadedTimeInMs: 222,
-                pageLoadedSizes: {0: 100},
-                numExecutions: 1000,
-              },
-              executionDigests: {},
-              pageSize: 100,
-              displayCount: 50,
-              scrollBeginIndex: 90,
-              focusIndex: 98,
-              executionData: {
-                98: createTestExecutionData({
-                  stack_frame_ids: ['a0', 'a1', 'a2'],
-                }),
-              },
-            },
-            stackFrames: {
-              a0: stackFrame0,
-              a1: stackFrame1,
-              a2: stackFrame2,
-            },
-          })
-        )
-      );
+      store.overrideSelector(getFocusedExecutionStackFrames, [
+        stackFrame0,
+        stackFrame1,
+        stackFrame2,
+      ]);
+      store.overrideSelector(getFocusedSourceLineSpec, {
+        host_name: stackFrame1[0],
+        file_path: stackFrame1[1],
+        lineno: stackFrame1[2],
+      });
       fixture.detectChanges();
 
       const hostNameElement = fixture.debugElement.query(
         By.css('.stack-trace-host-name')
       );
-      expect(hostNameElement.nativeElement.innerText).toEqual('(on localhost)');
+      expect(hostNameElement.nativeElement.innerText).toBe('(on localhost)');
       const stackFrameContainers = fixture.debugElement.queryAll(
         By.css('.stack-frame-container')
       );
-      expect(stackFrameContainers.length).toEqual(3);
+      expect(stackFrameContainers.length).toBe(3);
 
       const filePathElements = fixture.debugElement.queryAll(
         By.css('.stack-frame-file-path')
       );
-      expect(filePathElements.length).toEqual(3);
-      expect(filePathElements[0].nativeElement.innerText).toEqual(
+      expect(filePathElements.length).toBe(3);
+      expect(filePathElements[0].nativeElement.innerText).toBe(
         stackFrame0[1].slice(stackFrame0[1].lastIndexOf('/') + 1)
       );
-      expect(filePathElements[0].nativeElement.title).toEqual(stackFrame0[1]);
-      expect(filePathElements[1].nativeElement.innerText).toEqual(
+      expect(filePathElements[0].nativeElement.title).toBe(stackFrame0[1]);
+      expect(filePathElements[1].nativeElement.innerText).toBe(
         stackFrame1[1].slice(stackFrame1[1].lastIndexOf('/') + 1)
       );
-      expect(filePathElements[1].nativeElement.title).toEqual(stackFrame1[1]);
-      expect(filePathElements[2].nativeElement.innerText).toEqual(
+      expect(filePathElements[1].nativeElement.title).toBe(stackFrame1[1]);
+      expect(filePathElements[2].nativeElement.innerText).toBe(
         stackFrame2[1].slice(stackFrame2[1].lastIndexOf('/') + 1)
       );
-      expect(filePathElements[2].nativeElement.title).toEqual(stackFrame2[1]);
+      expect(filePathElements[2].nativeElement.title).toBe(stackFrame2[1]);
 
       const linenoElements = fixture.debugElement.queryAll(
         By.css('.stack-frame-lineno')
       );
-      expect(linenoElements.length).toEqual(3);
-      expect(linenoElements[0].nativeElement.innerText).toEqual(
+      expect(linenoElements.length).toBe(3);
+      expect(linenoElements[0].nativeElement.innerText).toBe(
         `Line ${stackFrame0[2]}`
       );
-      expect(linenoElements[1].nativeElement.innerText).toEqual(
+      expect(linenoElements[1].nativeElement.innerText).toBe(
         `Line ${stackFrame1[2]}`
       );
-      expect(linenoElements[2].nativeElement.innerText).toEqual(
+      expect(linenoElements[2].nativeElement.innerText).toBe(
         `Line ${stackFrame2[2]}`
       );
 
       const functionElements = fixture.debugElement.queryAll(
         By.css('.stack-frame-function')
       );
-      expect(functionElements.length).toEqual(3);
-      expect(functionElements[0].nativeElement.innerText).toEqual(
-        stackFrame0[3]
+      expect(functionElements.length).toBe(3);
+      expect(functionElements[0].nativeElement.innerText).toBe(stackFrame0[3]);
+      expect(functionElements[1].nativeElement.innerText).toBe(stackFrame1[3]);
+      expect(functionElements[2].nativeElement.innerText).toBe(stackFrame2[3]);
+
+      // Check the focused stack frame has been highlighted by CSS class.
+      const focusedElements = fixture.debugElement.queryAll(
+        By.css('.focused-stack-frame')
       );
-      expect(functionElements[1].nativeElement.innerText).toEqual(
-        stackFrame1[3]
+      expect(focusedElements.length).toBe(1);
+      const focusedFilePathElement = focusedElements[0].query(
+        By.css('.stack-frame-file-path')
       );
-      expect(functionElements[2].nativeElement.innerText).toEqual(
-        stackFrame2[3]
+      expect(focusedFilePathElement.nativeElement.innerText).toBe(
+        stackFrame1[1].slice(stackFrame1[1].lastIndexOf('/') + 1)
       );
     });
 
-    it('Shows loading state when stack-trace data is unavailable', () => {
+    it('does not highlight any frame when there is no frame focus', () => {
       const fixture = TestBed.createComponent(StackTraceContainer);
+      const stackFrame0 = createTestStackFrame();
+      const stackFrame1 = createTestStackFrame();
+      const stackFrame2 = createTestStackFrame();
+      store.overrideSelector(getFocusedExecutionStackFrames, [
+        stackFrame0,
+        stackFrame1,
+        stackFrame2,
+      ]);
+      store.overrideSelector(getFocusedSourceLineSpec, null);
       fixture.detectChanges();
 
-      store.setState(
-        createState(
-          createDebuggerState({
-            executions: {
-              numExecutionsLoaded: {
-                state: DataLoadState.LOADED,
-                lastLoadedTimeInMs: 111,
-              },
-              executionDigestsLoaded: {
-                state: DataLoadState.LOADED,
-                lastLoadedTimeInMs: 222,
-                pageLoadedSizes: {0: 100},
-                numExecutions: 1000,
-              },
-              executionDigests: {},
-              pageSize: 100,
-              displayCount: 50,
-              scrollBeginIndex: 90,
-              focusIndex: 98,
-              executionData: {
-                98: createTestExecutionData({
-                  stack_frame_ids: ['a0', 'a1', 'a2'],
-                }),
-              },
-            },
-            stackFrames: {}, // Note the empty stackFrames field.
-          })
-        )
+      // Check that no stack frame has been highlighted by CSS class.
+      const focusedElements = fixture.debugElement.queryAll(
+        By.css('.focused-stack-frame')
       );
+      expect(focusedElements.length).toBe(0);
+    });
+
+    it('Shows loading state when stack-trace data is unavailable', () => {
+      const fixture = TestBed.createComponent(StackTraceContainer);
+      store.overrideSelector(getFocusedExecutionStackFrames, []);
       fixture.detectChanges();
 
       const stackFrameContainers = fixture.debugElement.queryAll(
         By.css('.stack-frame-container')
       );
-      expect(stackFrameContainers.length).toEqual(0);
+      expect(stackFrameContainers.length).toBe(0);
     });
 
     it('Emits sourceLineFocused when line number is clicked', () => {
       const fixture = TestBed.createComponent(StackTraceContainer);
-      fixture.detectChanges();
-
       const stackFrame0 = createTestStackFrame();
       const stackFrame1 = createTestStackFrame();
       const stackFrame2 = createTestStackFrame();
-      store.setState(
-        createState(
-          createDebuggerState({
-            executions: {
-              numExecutionsLoaded: {
-                state: DataLoadState.LOADED,
-                lastLoadedTimeInMs: 111,
-              },
-              executionDigestsLoaded: {
-                state: DataLoadState.LOADED,
-                lastLoadedTimeInMs: 222,
-                pageLoadedSizes: {0: 100},
-                numExecutions: 1000,
-              },
-              executionDigests: {},
-              pageSize: 100,
-              displayCount: 50,
-              scrollBeginIndex: 90,
-              focusIndex: 98,
-              executionData: {
-                98: createTestExecutionData({
-                  stack_frame_ids: ['a0', 'a1', 'a2'],
-                }),
-              },
-            },
-            stackFrames: {
-              a0: stackFrame0,
-              a1: stackFrame1,
-              a2: stackFrame2,
-            },
-          })
-        )
-      );
+      store.overrideSelector(getFocusedExecutionStackFrames, [
+        stackFrame0,
+        stackFrame1,
+        stackFrame2,
+      ]);
       fixture.detectChanges();
 
       const linenoElements = fixture.debugElement.queryAll(

tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/store/debugger_types.ts

@@ -44,6 +44,8 @@ export interface DebuggerRunListing {
 }
 
 // Each item is [host_name, file_path, lineno, function].
+// TODO(cais): Consider making this an object with meaningful
+// keys instead.
 export type StackFrame = [string, string, number, string];
 
 export type StackFramesById = {[id: string]: StackFrame};

tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/stack_trace/stack_trace_component.css

@@ -13,6 +13,11 @@ See the License for the specific language governing permissions and
 limitations under the License.
 ==============================================================================*/
 
+.focused-stack-frame {
+  background-color: rgba(255, 111, 0, 0.3);
+  font-weight: bold;
+}
+
 .stack-frame-array {
   height: 360px;
   overflow-x: auto;

tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/stack_trace/stack_trace_component.ng.html

@@ -35,7 +35,10 @@
     <div
       *ngFor="let stackFrameForDisplay of stackFramesForDisplay; index as i;"
     >
-      <div class="stack-frame-container">
+      <div
+        class="stack-frame-container"
+        [ngClass]="[stackFrameForDisplay.focused ? 'focused-stack-frame' : '']"
+      >
         <div
           class="stack-frame-file-path"
           title="{{stackFrameForDisplay.file_path}}"

tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/stack_trace/stack_trace_component.ts

@@ -20,6 +20,9 @@ export interface StackFrameForDisplay {
   concise_file_path: string;
   lineno: number;
   function_name: string;
+  // Whether the stack frame is being focused on (e.g.,
+  // being viewed in the source code viewer).
+  focused: boolean;
 }
 
 @Component({

tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/stack_trace/stack_trace_container.ts

@@ -18,7 +18,10 @@ import {createSelector, select, Store} from '@ngrx/store';
 import {State} from '../../store/debugger_types';
 
 import {sourceLineFocused} from '../../actions';
-import {getFocusedExecutionStackFrames} from '../../store';
+import {
+  getFocusedExecutionStackFrames,
+  getFocusedSourceLineSpec,
+} from '../../store';
 import {StackFrameForDisplay} from './stack_trace_component';
 
 /** @typehack */ import * as _typeHackRxjs from 'rxjs';
@@ -37,7 +40,8 @@ export class StackTraceContainer {
     select(
       createSelector(
         getFocusedExecutionStackFrames,
-        (stackFrames) => {
+        getFocusedSourceLineSpec,
+        (stackFrames, focusedSourceLineSpec) => {
           if (stackFrames === null) {
             return null;
           }
@@ -46,12 +50,18 @@ export class StackTraceContainer {
             const [host_name, file_path, lineno, function_name] = stackFrame;
             const pathItems = file_path.split('/');
             const concise_file_path = pathItems[pathItems.length - 1];
+            const focused =
+              focusedSourceLineSpec !== null &&
+              host_name === focusedSourceLineSpec.host_name &&
+              file_path === focusedSourceLineSpec.file_path &&
+              lineno === focusedSourceLineSpec.lineno;
             output.push({
               host_name,
               file_path,
               concise_file_path,
               lineno,
               function_name,
+              focused,
             });
           }
           return output;