Skip to content

Commit

Permalink
Refactoring: DAG migrated to Plexus's Digraph (#1981)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- This issue is related to LFX task #1466 
- I can create a more specific sub-issue if that's more suitable

## Description of the changes
- I've removed `cytoscape` and replaced it with `Digraph` from `Plexus`
- Had to add support for labels in `Digraph` to support this change

## How was this change tested?
- The changes were tested visually
- Tested with test data involving around 50 nodes

Before - 

<img width="1785" alt="Screenshot 2023-11-18 at 22 06 10"
src="https://github.com/jaegertracing/jaeger-ui/assets/28570857/dd130e06-94d5-41b8-8217-7e0cdb442dc2">

Now -

<img width="1785" alt="Screenshot 2023-11-18 at 22 06 22"
src="https://github.com/jaegertracing/jaeger-ui/assets/28570857/6249207d-b1e6-4774-82ad-2f9e6a589f3e">

Tha additional tab visible in above images has been removed

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Prathamesh Mutkure <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
  • Loading branch information
3 people authored Dec 18, 2023
1 parent 63361a2 commit 2947a10
Show file tree
Hide file tree
Showing 11 changed files with 332 additions and 173 deletions.
3 changes: 0 additions & 3 deletions packages/jaeger-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@
"classnames": "^2.2.5",
"combokeys": "^3.0.0",
"copy-to-clipboard": "^3.1.0",
"cytoscape": "^3.2.1",
"cytoscape-dagre": "^2.0.0",
"dagre": "^0.8.5",
"dayjs": "^1.11.9",
"deep-freeze": "^0.0.1",
"drange": "^2.0.0",
Expand Down
118 changes: 0 additions & 118 deletions packages/jaeger-ui/src/components/DependencyGraph/DAG.jsx

This file was deleted.

112 changes: 105 additions & 7 deletions packages/jaeger-ui/src/components/DependencyGraph/DAG.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
// limitations under the License.

import React from 'react';
import ShallowRenderer from 'react-test-renderer/shallow';
import { render, screen } from '@testing-library/react';
import DAG from './DAG';
import { shallow } from 'enzyme';
import DAG, { renderNode } from './DAG';

// mock canvas API (we don't care about canvas results)

Expand Down Expand Up @@ -54,28 +56,124 @@ window.HTMLCanvasElement.prototype.getContext = function getContext() {
clip() {},
};
};

describe('<DAG>', () => {
it('does not explode', () => {
let renderer;

beforeEach(() => {
renderer = new ShallowRenderer();
});

it('shows correct number of nodes and vertices', () => {
const serviceCalls = [
{
callCount: 1,
child: 'child-id',
parent: 'parent-id',
},
];
render(<DAG serviceCalls={serviceCalls} />);
expect(screen.getByTestId('cy')).toBeDefined();

renderer.render(<DAG serviceCalls={serviceCalls} />);
const element = renderer.getRenderOutput();

expect(element.props.children.props.vertices.length).toBe(2);
expect(element.props.children.props.edges.length).toBe(1);
});

it('does not explode with empty strings or string with only spaces', () => {
it('does not show nodes with empty strings or string with only spaces', () => {
const serviceCalls = [
{
callCount: 1,
child: '',
parent: ' ',
},
];
render(<DAG serviceCalls={serviceCalls} />);
expect(screen.getByTestId('cy')).toBeDefined();

renderer.render(<DAG serviceCalls={serviceCalls} />);
const element = renderer.getRenderOutput();

// Empty or blank strings getting skipped is desirable
// But should not cause the component to break
expect(element.props.children.props.vertices.length).toBe(0);
expect(element.props.children.props.edges.length).toBe(0);
});
});

describe('renderNode', () => {
it('correctly displays the vertex key', async () => {
const vertex = {
key: 'Test',
};

render(renderNode(vertex));

const element = await screen.findByTestId('dagNodeLabel');

expect(element.textContent).toBe('Test');
});

it('correctly displays the vertex key if it is number', async () => {
const vertex = {
key: 2,
};

render(renderNode(vertex));

const element = await screen.findByTestId('dagNodeLabel');

expect(element.textContent).toBe('2');
});

it('displays nothing if vertext is undefined', async () => {
const vertex = undefined;

render(renderNode(vertex));

const element = await screen.findByTestId('dagNodeLabel');

expect(element.textContent).toBe('');
});

it('displays nothing if vertext is null', async () => {
const vertex = null;

render(renderNode(vertex));

const element = await screen.findByTestId('dagNodeLabel');

expect(element.textContent).toBe('');
});

it('displays nothing if vertext key is undefined', async () => {
const vertex = {
key: undefined,
};

render(renderNode(vertex));

const element = await screen.findByTestId('dagNodeLabel');

expect(element.textContent).toBe('');
});

it('displays nothing if vertext key is null', async () => {
const vertex = {
key: null,
};

render(renderNode(vertex));

const element = await screen.findByTestId('dagNodeLabel');

expect(element.textContent).toBe('');
});
});

describe('clean up', () => {
it('stops LayoutManager before unmounting', () => {
const wrapper = shallow(<DAG serviceCalls={[]} />);
const stopAndReleaseSpy = jest.spyOn(wrapper.instance().layoutManager, 'stopAndRelease');
wrapper.unmount();
expect(stopAndReleaseSpy).toHaveBeenCalledTimes(1);
});
});
Loading

0 comments on commit 2947a10

Please sign in to comment.