Skip to content

test: add tests for MetricChart component - #54

Open
mrhapile wants to merge 1 commit into
inspektor-gadget:mainfrom
mrhapile:test/metric-chart
Open

test: add tests for MetricChart component#54
mrhapile wants to merge 1 commit into
inspektor-gadget:mainfrom
mrhapile:test/metric-chart

Conversation

@mrhapile

@mrhapile mrhapile commented Mar 7, 2026

Copy link
Copy Markdown

Add tests for MetricChart component

ref #20

This PR adds a test file for the MetricChart component located in src/common/MetricChart/index.tsx.

The tests verify that the component renders correctly and that the chart container appears when the component is mounted. The tests also check that the component handles empty data and renders properly when valid metric data is provided.

How to use

Reviewers can validate this PR by running the test suite locally.

Steps:

  1. Checkout this branch.
  2. Install dependencies if needed.
  3. Run the test suite.

Testing done

Commands executed:

npm install
npm test

Result:

Screenshot 2026-03-08 at 1 13 30 AM

All tests passed successfully and the new test file src/common/MetricChart/index.test.tsx executed without errors.

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>
Copilot AI review requested due to automatic review settings March 7, 2026 19:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a test file for the MetricChart component (src/common/MetricChart/index.tsx), which renders a bar chart using Recharts and MUI components. The tests verify both the non-rendering (empty/invalid data) and rendering (valid data) behaviors of the component, as part of the broader test coverage effort tracked in issue #20.

Changes:

  • New test file src/common/MetricChart/index.test.tsx with 7 test cases covering empty data, null data, invalid field configurations, and correct chart rendering with axis labels.
  • Mocks for recharts components and @kinvolk/headlamp-plugin/lib/components/common SectionBox are included to isolate the component under test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +67 to +71
expect(screen.getByTestId('section-box')).toBeDefined();
expect(screen.getByTestId('section-box').getAttribute('data-title')).toBe('Metric Chart for node node-test');

expect(screen.getByTestId('responsive-container')).toBeDefined();
expect(screen.getByTestId('bar-chart')).toBeDefined();

Copilot AI Mar 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertions expect(screen.getByTestId('section-box')).toBeDefined(), expect(screen.getByTestId('responsive-container')).toBeDefined(), and expect(screen.getByTestId('bar-chart')).toBeDefined() are redundant. screen.getByTestId() already throws an error if the element is not found — it never returns undefined. The .toBeDefined() check adds no additional verification. Since the intent is to assert that the elements are present in the DOM, simply calling screen.getByTestId(...) (without wrapping in expect(...).toBeDefined()) is sufficient — if the element is absent, the test will still fail with a descriptive error from RTL.

Suggested change
expect(screen.getByTestId('section-box')).toBeDefined();
expect(screen.getByTestId('section-box').getAttribute('data-title')).toBe('Metric Chart for node node-test');
expect(screen.getByTestId('responsive-container')).toBeDefined();
expect(screen.getByTestId('bar-chart')).toBeDefined();
const sectionBox = screen.getByTestId('section-box');
expect(sectionBox.getAttribute('data-title')).toBe('Metric Chart for node node-test');
screen.getByTestId('responsive-container');
screen.getByTestId('bar-chart');

Copilot uses AI. Check for mistakes.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +8 to +12
// Mock recharts
vi.mock('recharts', () => ({
BarChart: ({ children }: any) => <div data-testid="bar-chart">{children}</div>,
Bar: () => <div data-testid="bar" />,
CartesianGrid: () => <div data-testid="cartesian-grid" />,

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation in this new test file uses 4 spaces in many blocks, but the surrounding codebase uses 2-space indentation (e.g., src/common/MetricChart/index.tsx and src/common/gadgetbackgroundinstanceform.tsx). Please run the repo formatter (prettier/headlamp-plugin format) so this file matches the established formatting and avoids noisy diffs or potential lint/format CI failures.

Copilot uses AI. Check for mistakes.

@illume illume left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contributions!

Please let us know if you want to continue this by addressing the review comments? If not that's ok, we can take over the PR (and finish it or close it ourselves).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants