Clean up, TODO: track

Signed-off-by: Everett Ross <[email protected]>
vvvprabhakar · Nov 22, 2019 · ff7b6e4 · ff7b6e4
1 parent c33d4ab
commit ff7b6e4
Show file tree

Hide file tree

Showing 13 changed files with 69 additions and 76 deletions.
diff --git a/...-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap b/...-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap
@@ -3,8 +3,8 @@
 exports[`<DdgNodeContent> DdgNodeContent.getNodeRenderer() returns a <DdgNodeContent /> 1`] = `
 <div
   className="DdgNodeContent"
-  onMouseEnter={[Function]}
-  onMouseLeave={[Function]}
+  onMouseOut={[Function]}
+  onMouseOver={[Function]}
 >
   <div
     className="DdgNodeContent--core"
@@ -152,8 +152,8 @@ exports[`<DdgNodeContent> DdgNodeContent.getNodeRenderer() returns a <DdgNodeCon
 exports[`<DdgNodeContent> DdgNodeContent.getNodeRenderer() returns a focal <DdgNodeContent /> 1`] = `
 <div
   className="DdgNodeContent"
-  onMouseEnter={[Function]}
-  onMouseLeave={[Function]}
+  onMouseOut={[Function]}
+  onMouseOver={[Function]}
 >
   <div
     className="DdgNodeContent--core is-focalNode"
@@ -228,8 +228,8 @@ exports[`<DdgNodeContent> DdgNodeContent.getNodeRenderer() returns a focal <DdgN
 exports[`<DdgNodeContent> omits the operation if it is null 1`] = `
 <div
   className="DdgNodeContent"
-  onMouseEnter={[Function]}
-  onMouseLeave={[Function]}
+  onMouseOut={[Function]}
+  onMouseOver={[Function]}
 >
   <div
     className="DdgNodeContent--core"
@@ -377,8 +377,8 @@ exports[`<DdgNodeContent> omits the operation if it is null 1`] = `
 exports[`<DdgNodeContent> omits the operation if it is null 2`] = `
 <div
   className="DdgNodeContent"
-  onMouseEnter={[Function]}
-  onMouseLeave={[Function]}
+  onMouseOut={[Function]}
+  onMouseOver={[Function]}
 >
   <div
     className="DdgNodeContent--core"
@@ -511,8 +511,8 @@ exports[`<DdgNodeContent> omits the operation if it is null 2`] = `
 exports[`<DdgNodeContent> renders correctly when isFocalNode = true and focalNodeUrl = null 1`] = `
 <div
   className="DdgNodeContent"
-  onMouseEnter={[Function]}
-  onMouseLeave={[Function]}
+  onMouseOut={[Function]}
+  onMouseOver={[Function]}
 >
   <div
     className="DdgNodeContent--core"
@@ -660,8 +660,8 @@ exports[`<DdgNodeContent> renders correctly when isFocalNode = true and focalNod
 exports[`<DdgNodeContent> renders correctly when isFocalNode = true and focalNodeUrl = null 2`] = `
 <div
   className="DdgNodeContent"
-  onMouseEnter={[Function]}
-  onMouseLeave={[Function]}
+  onMouseOut={[Function]}
+  onMouseOver={[Function]}
 >
   <div
     className="DdgNodeContent--core is-focalNode"
@@ -736,8 +736,8 @@ exports[`<DdgNodeContent> renders correctly when isFocalNode = true and focalNod
 exports[`<DdgNodeContent> renders the number of operations if there are multiple 1`] = `
 <div
   className="DdgNodeContent"
-  onMouseEnter={[Function]}
-  onMouseLeave={[Function]}
+  onMouseOut={[Function]}
+  onMouseOver={[Function]}
 >
   <div
     className="DdgNodeContent--core"
@@ -885,8 +885,8 @@ exports[`<DdgNodeContent> renders the number of operations if there are multiple
 exports[`<DdgNodeContent> renders the number of operations if there are multiple 2`] = `
 <div
   className="DdgNodeContent"
-  onMouseEnter={[Function]}
-  onMouseLeave={[Function]}
+  onMouseOut={[Function]}
+  onMouseOver={[Function]}
 >
   <div
     className="DdgNodeContent--core"

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.css b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.css
@@ -77,7 +77,7 @@ limitations under the License.
   transition-property: opacity;
 }
 
-.DdgNodeContent:hover .DdgNodeContent--actionsWrapper {
+.DdgNodeContent:hover > .DdgNodeContent--actionsWrapper {
   opacity: 1;
   pointer-events: all;
   transition-delay: 0s;

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js
@@ -97,26 +97,26 @@ describe('<DdgNodeContent>', () => {
       props.getVisiblePathElems.mockReturnValue(testElems);
     });
 
-    it('calls setViewModifier on mouse enter', () => {
-      wrapper.simulate('mouseenter', { type: 'mouseenter' });
+    it('calls setViewModifier on mouse over', () => {
+      wrapper.simulate('mouseover', { type: 'mouseover' });
 
       expect(props.setViewModifier).toHaveBeenCalledTimes(1);
       expect(props.setViewModifier).toHaveBeenCalledWith(testIndices, EViewModifier.Hovered, true);
     });
 
-    it('calls setViewModifier with all modified indices on mouse leave', () => {
-      wrapper.simulate('mouseenter', { type: 'mouseenter' });
-      wrapper.simulate('mouseleave', { type: 'mouseleave' });
+    it('calls setViewModifier with all modified indices on mouse out', () => {
+      wrapper.simulate('mouseover', { type: 'mouseover' });
+      wrapper.simulate('mouseout', { type: 'mouseout' });
 
       expect(props.setViewModifier).toHaveBeenCalledTimes(2);
       expect(props.setViewModifier).toHaveBeenCalledWith(testIndices, EViewModifier.Hovered, false);
 
-      wrapper.simulate('mouseenter', { type: 'mouseenter' });
+      wrapper.simulate('mouseover', { type: 'mouseover' });
       const moreIndices = [108];
       const moreElems = moreIndices.map(visibilityIdx => ({ visibilityIdx }));
       props.getVisiblePathElems.mockReturnValue(moreElems);
-      wrapper.simulate('mouseenter', { type: 'mouseenter' });
-      wrapper.simulate('mouseleave', { type: 'mouseleave' });
+      wrapper.simulate('mouseover', { type: 'mouseover' });
+      wrapper.simulate('mouseout', { type: 'mouseout' });
 
       expect(props.setViewModifier).toHaveBeenCalledTimes(5);
       expect(props.setViewModifier).toHaveBeenCalledWith(
@@ -131,27 +131,27 @@ describe('<DdgNodeContent>', () => {
       expect(props.setViewModifier).toHaveBeenCalledTimes(0);
 
       wrapper = shallow(<DdgNodeContent {...props} />);
-      wrapper.simulate('mouseenter', { type: 'mouseenter' });
-      wrapper.simulate('mouseleave', { type: 'mouseleave' });
+      wrapper.simulate('mouseover', { type: 'mouseover' });
+      wrapper.simulate('mouseout', { type: 'mouseout' });
       expect(props.setViewModifier).toHaveBeenCalledTimes(2);
       wrapper.unmount();
       expect(props.setViewModifier).toHaveBeenCalledTimes(2);
 
       wrapper = shallow(<DdgNodeContent {...props} />);
-      wrapper.simulate('mouseenter', { type: 'mouseenter' });
+      wrapper.simulate('mouseover', { type: 'mouseover' });
+      expect(props.setViewModifier).toHaveBeenCalledTimes(3);
       wrapper.unmount();
-
       expect(props.setViewModifier).toHaveBeenCalledTimes(4);
       expect(props.setViewModifier).toHaveBeenCalledWith(testIndices, EViewModifier.Hovered, false);
     });
 
-    it('calculates state.childrenVisibility and state.parentVisibility on mouse enter', () => {
+    it('calculates state.childrenVisibility and state.parentVisibility on mouse over', () => {
       const childrenVisibility = ECheckedStatus.Partial;
       const parentVisibility = ECheckedStatus.Full;
       props.getGenerationVisibility.mockImplementation((_key, direction) =>
         direction === EDirection.Upstream ? parentVisibility : childrenVisibility
       );
-      wrapper.simulate('mouseenter', { type: 'mouseenter' });
+      wrapper.simulate('mouseover', { type: 'mouseover' });
 
       expect(wrapper.state()).toEqual({
         childrenVisibility,

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx
@@ -190,7 +190,7 @@ export default class DdgNodeContent extends React.PureComponent<TProps, TState>
 
   private onMouseUx = (event: React.MouseEvent<HTMLElement>) => {
     const { getGenerationVisibility, getVisiblePathElems, setViewModifier, vertexKey } = this.props;
-    const hovered = event.type === 'mouseenter';
+    const hovered = event.type === 'mouseover';
     const visIndices = hovered
       ? (getVisiblePathElems(vertexKey) || []).map(({ visibilityIdx }) => {
           this.hoveredIndices.add(visibilityIdx);
@@ -216,7 +216,7 @@ export default class DdgNodeContent extends React.PureComponent<TProps, TState>
     const transform = `translate(${RADIUS - radius}px, ${RADIUS - radius}px) scale(${scaleFactor})`;
 
     return (
-      <div className="DdgNodeContent" onMouseEnter={this.onMouseUx} onMouseLeave={this.onMouseUx}>
+      <div className="DdgNodeContent" onMouseOver={this.onMouseUx} onMouseOut={this.onMouseUx}>
         <div
           className={cx('DdgNodeContent--core', {
             'is-focalNode': isFocalNode,

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.test.js b/packages/jaeger-ui/src/components/DeepDependencies/Header/NameSelector.test.js
@@ -147,8 +147,8 @@ describe('<NameSelector>', () => {
       wrapper.find(Icon).simulate('click', { stopPropagation });
 
       expect(clearValue).toHaveBeenCalled();
-      expect(stopPropagation).toHaveBeenCalled();
       expect(wrapper.state('popoverVisible')).toBe(false);
+      expect(stopPropagation).toHaveBeenCalled();
     });
 
     it('throws Error when attempting to clear when required', () => {

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/index.test.js.snap b/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/index.test.js.snap
@@ -77,6 +77,7 @@ exports[`<Header> renders the operation selector if a service is selected 1`] =
       value="testService"
     />
     <NameSelector
+      clearValue={[Function]}
       label="Operation"
       options={Array []}
       placeholder="Filter by operation…"
@@ -143,6 +144,7 @@ exports[`<Header> renders the operation selector if a service is selected 2`] =
       value="testService"
     />
     <NameSelector
+      clearValue={[Function]}
       label="Operation"
       options={
         Array [

diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.test.js
@@ -24,6 +24,7 @@ import NameSelector from './NameSelector';
 
 describe('<Header>', () => {
   const minProps = {
+    clearOperation: () => {},
     setDistance: () => {},
     setOperation: () => {},
     setService: () => {},
@@ -116,6 +117,7 @@ describe('<Header>', () => {
 
     it('renders both visible and hidden counts if both are provided', () => {
       wrapper.setProps({ hiddenUiFindMatches, uiFindCount });
+
       expect(getMatchesInfo().text()).toEqual(expectedFindCount);
       expect(getMatchesInfo().text()).toEqual(expectedHiddenCount);
       expect(getTooltip().prop('title')).toBe(expectedHiddenTitle);
@@ -124,17 +126,21 @@ describe('<Header>', () => {
       expect(wrapper.find(MdVisibilityOff)).toHaveLength(1);
     });
 
-    it('renders 0 with correct tooltip if there are no matches', () => {
+    it('renders 0 with correct tooltip if there are no visible nor hidden matches', () => {
       const expectedTitle = 'No matches';
-
       wrapper.setProps({ uiFindCount: 0 });
+
       expect(getMatchesInfo().text()).toBe('0');
       expect(getTooltip().prop('title')).toBe(expectedTitle);
       expect(getBtn().prop('disabled')).toBe(true);
       expect(wrapper.find(MdVisibility)).toHaveLength(0);
       expect(wrapper.find(MdVisibilityOff)).toHaveLength(0);
+    });
 
+    it('renders 0 with correct tooltip if there are no matches but there are hidden matches', () => {
+      wrapper.setProps({ uiFindCount: 0 });
       wrapper.setProps({ hiddenUiFindMatches });
+
       expect(getMatchesInfo().text()).toEqual(expect.stringContaining('0'));
       expect(getMatchesInfo().text()).toEqual(expectedHiddenCount);
       expect(getTooltip().prop('title')).toBe(expectedHiddenTitle);
@@ -144,9 +150,9 @@ describe('<Header>', () => {
     });
 
     it('renders correct plurality in tooltip', () => {
-      const expectedTitle = `Click to view 1 hidden match`;
-
+      const expectedTitle = 'Click to view 1 hidden match';
       wrapper.setProps({ hiddenUiFindMatches: new Set([Array.from(hiddenUiFindMatches)[0]]), uiFindCount });
+
       expect(getTooltip().prop('title')).toBe(expectedTitle);
     });
 

diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js
@@ -154,7 +154,7 @@ describe('lookback utils', () => {
       });
     });
 
-    // DST/not-DST means this test will fail four 14 out of 52 weeks of the year!
+    // DST/not-DST means this test will fail for 14 out of 52 weeks of the year!
     xit('creates timestamp for weeks ago', () => {
       [1, 2, 4, 7].forEach(lookbackNum => {
         expect(nowInMicroseconds - lookbackToTimestamp(`${lookbackNum}w`, now)).toBe(

diff --git a/packages/jaeger-ui/src/components/common/FilteredList/index.test.js b/packages/jaeger-ui/src/components/common/FilteredList/index.test.js
@@ -140,8 +140,8 @@ describe('<FilteredList>', () => {
 
   it('enter selects the filteredOption if there is only one option', () => {
     const value = words[1];
-    expect(props.setValue.mock.calls.length).toBe(0);
     wrapper.find('input').simulate('change', { target: { value } });
+    expect(props.setValue.mock.calls.length).toBe(0);
     keyDown(EKey.Enter);
     expect(props.setValue.mock.calls).toEqual([[value]]);
   });

diff --git a/packages/jaeger-ui/src/model/ddg/GraphModel/index.test.js b/packages/jaeger-ui/src/model/ddg/GraphModel/index.test.js
@@ -255,7 +255,7 @@ describe('GraphModel', () => {
       ...downstreamTargets.map(getIdx),
       ...upstreamTargets.map(getIdx),
     ];
-    const allButSomeExternalVisible = encode([
+    const allButSomeExternalVisEncoding = encode([
       ...twoHopIndices,
       ...subsetOfTargetExternalNeighborsVisibilityIndices,
     ]);
@@ -360,6 +360,7 @@ describe('GraphModel', () => {
             partialTargetWithRespectiveExternalVisEncoding
           )
         ).toEqual(ECheckedStatus.Full);
+
         expect(generationGraph.getGenerationVisibility(targetKey, EDirection.Upstream, allVisible)).toEqual(
           ECheckedStatus.Full
         );
@@ -374,11 +375,19 @@ describe('GraphModel', () => {
 
       it('returns ECheckedStatus.Partial if only some neighbors are visible', () => {
         expect(
-          generationGraph.getGenerationVisibility(targetKey, EDirection.Downstream, allButSomeExternalVisible)
+          generationGraph.getGenerationVisibility(
+            targetKey,
+            EDirection.Downstream,
+            allButSomeExternalVisEncoding
+          )
         ).toEqual(ECheckedStatus.Partial);
 
         expect(
-          generationGraph.getGenerationVisibility(targetKey, EDirection.Upstream, allButSomeExternalVisible)
+          generationGraph.getGenerationVisibility(
+            targetKey,
+            EDirection.Upstream,
+            allButSomeExternalVisEncoding
+          )
         ).toEqual(ECheckedStatus.Partial);
       });
     });
@@ -448,7 +457,7 @@ describe('GraphModel', () => {
           generationGraph.getVisWithUpdatedGeneration(
             targetKey,
             EDirection.Downstream,
-            allButSomeExternalVisible
+            allButSomeExternalVisEncoding
           )
         ).toEqual({
           visEncoding: encode([
@@ -462,7 +471,7 @@ describe('GraphModel', () => {
           generationGraph.getVisWithUpdatedGeneration(
             targetKey,
             EDirection.Upstream,
-            allButSomeExternalVisible
+            allButSomeExternalVisEncoding
           )
         ).toEqual({
           visEncoding: encode([...upstreamFullIndices, ...subsetOfTargetExternalNeighborsVisibilityIndices]),
@@ -531,7 +540,7 @@ describe('GraphModel', () => {
       });
 
       it('returns focal vertex with multiple operations', () => {
-        const { vertices } = withoutFocalOpGraph.getVisible();
+        const { vertices } = withoutFocalOpGraph.getVisible(encode([0, 1]));
         const focalOps = vertices[vertices.length - 1].operation;
 
         expect(focalOps).toEqual(expect.any(Array));

diff --git a/packages/jaeger-ui/src/model/ddg/GraphModel/index.tsx b/packages/jaeger-ui/src/model/ddg/GraphModel/index.tsx
@@ -156,16 +156,10 @@ export default class GraphModel {
     if (!generation.length) return null;
 
     const visibleIndices = this.getVisibleIndices(visEncoding);
-    let partial = false;
-    let full = true;
-    generation.forEach(elem => {
-      const isVis = visibleIndices.has(elem.visibilityIdx);
-      partial = partial || isVis;
-      full = full && isVis;
-    });
+    const visibleGeneration = generation.filter(({ visibilityIdx }) => visibleIndices.has(visibilityIdx));
 
-    if (full) return ECheckedStatus.Full;
-    if (partial) return ECheckedStatus.Partial;
+    if (visibleGeneration.length === generation.length) return ECheckedStatus.Full;
+    if (visibleGeneration.length) return ECheckedStatus.Partial;
     return ECheckedStatus.Empty;
   };
 
@@ -189,7 +183,7 @@ export default class GraphModel {
         const edge = this.pathElemToEdge.get(pathElem);
         if (edge) edges.add(edge);
         const vertex = this.pathElemToVertex.get(pathElem);
-        if (vertex && vertex.key !== FOCAL_KEY) vertices.add(vertex);
+        if (vertex && !vertex.isFocalNode) vertices.add(vertex);
       });
 
       if (this.visIdxToPathElem.length) {

diff --git a/packages/jaeger-ui/src/model/ddg/PathElem.test.js b/packages/jaeger-ui/src/model/ddg/PathElem.test.js
@@ -132,13 +132,13 @@ describe('PathElem', () => {
       expect(targetPathElem.externalPath).toEqual([targetPathElem]);
     });
 
-    it('returns path to focal elem in correct order for upstream elem', () => {
+    it('returns path away from focal elem in correct order for upstream elem', () => {
       const idx = path.focalIdx - 1;
       const targetPathElem = path.members[idx];
       expect(targetPathElem.externalPath).toEqual(path.members.slice(0, idx + 1));
     });
 
-    it('returns path to focal elem in correct order for downstream elem', () => {
+    it('returns path away from focal elem in correct order for downstream elem', () => {
       const idx = path.focalIdx + 1;
       const targetPathElem = path.members[idx];
       expect(targetPathElem.externalPath).toEqual(path.members.slice(idx));