Skip to content

test: add tests for gadgetbackgroundinstanceform component - #55

Open
mrhapile wants to merge 1 commit into
inspektor-gadget:mainfrom
mrhapile:test/gadget-background-instance-form
Open

test: add tests for gadgetbackgroundinstanceform component#55
mrhapile wants to merge 1 commit into
inspektor-gadget:mainfrom
mrhapile:test/gadget-background-instance-form

Conversation

@mrhapile

@mrhapile mrhapile commented Mar 7, 2026

Copy link
Copy Markdown

Add tests for gadgetbackgroundinstanceform component

ref #20

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

The tests verify that the component renders correctly and that the form elements behave as expected when interacting with the UI. The tests follow the existing testing conventions used in the repository and help improve the overall test coverage.

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 2 01 39 AM

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

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

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

Adds initial UI test coverage for the GadgetBackgroundInstanceForm component (src/common/gadgetbackgroundinstanceform.tsx) as part of issue #20’s “Common components” testing checklist.

Changes:

  • Introduces a new Vitest + React Testing Library test suite for GadgetBackgroundInstanceForm.
  • Covers rendering, form interactions, localStorage persistence, and background-instance creation flows (success/error).
  • Adds mocking for Headlamp/K8s integrations and the gadget connection layer.

💡 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.

import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest';
import { GadgetBackgroundInstanceForm } from './gadgetbackgroundinstanceform';
import React from 'react';
import { useGadgetConn } from '../gadgets/conn';

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.

Unused import: useGadgetConn is imported but never referenced in this test file. This can trip TypeScript/ESLint noUnusedLocals depending on the repo config; please remove the import (the module is already mocked via vi.mock('../gadgets/conn', ...)).

Suggested change
import { useGadgetConn } from '../gadgets/conn';

Copilot uses AI. Check for mistakes.
Comment on lines +87 to +95
beforeEach(() => {
vi.clearAllMocks();
localStorage.clear();
});

afterEach(() => {
vi.clearAllMocks();
cleanup();
});

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.

vi.clearAllMocks() does not reset mock implementations/return values. Since some tests call mockUseGadgetConn.mockReturnValue(...), that override can leak into later tests and cause order-dependent failures. Reinitialize mockUseGadgetConn's return value in beforeEach (or use mockUseGadgetConn.mockReset() + re-mockReturnValue), so every test starts from the same default connection stub.

Copilot uses AI. Check for mistakes.
Comment on lines +323 to +350

test('displays validation error if name is empty (branch coverage)', () => {
render(<GadgetBackgroundInstanceForm {...defaultProps} />);

const nameInput = screen.getByLabelText(/Instance Name/i);
fireEvent.change(nameInput, { target: { value: '' } });

const createButton: any = screen.getByRole('button', { name: /create instance/i });

// Find internal React fiber node to trigger the bound onClick directly,
// bypassing MUI's native disabled block and testing-library's strict bounds.
const fiberKey = Object.keys(createButton).find(key => key.startsWith('__reactFiber$'));
let fiber = fiberKey ? createButton[fiberKey] : null;

let originalHandler = null;
while (fiber) {
if (fiber.memoizedProps && typeof fiber.memoizedProps.onClick === 'function') {
originalHandler = fiber.memoizedProps.onClick;
}
fiber = fiber.return;
}

if (originalHandler) {
originalHandler();
}

expect(mockEnqueueSnackbar).toHaveBeenCalledWith('Please fill all required fields', { variant: 'error' });
});

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 test triggers the click handler by walking React internal fiber fields (__reactFiber$...). This relies on private React implementation details and is very likely to break with React/MUI/Vitest upgrades. Since the button is intentionally disabled when the name is empty, this path is not user-reachable; consider removing this test (the earlier disabled-button assertion already covers the user-facing behavior) or refactoring the component to expose/test the validation logic without poking React internals.

Suggested change
test('displays validation error if name is empty (branch coverage)', () => {
render(<GadgetBackgroundInstanceForm {...defaultProps} />);
const nameInput = screen.getByLabelText(/Instance Name/i);
fireEvent.change(nameInput, { target: { value: '' } });
const createButton: any = screen.getByRole('button', { name: /create instance/i });
// Find internal React fiber node to trigger the bound onClick directly,
// bypassing MUI's native disabled block and testing-library's strict bounds.
const fiberKey = Object.keys(createButton).find(key => key.startsWith('__reactFiber$'));
let fiber = fiberKey ? createButton[fiberKey] : null;
let originalHandler = null;
while (fiber) {
if (fiber.memoizedProps && typeof fiber.memoizedProps.onClick === 'function') {
originalHandler = fiber.memoizedProps.onClick;
}
fiber = fiber.return;
}
if (originalHandler) {
originalHandler();
}
expect(mockEnqueueSnackbar).toHaveBeenCalledWith('Please fill all required fields', { variant: 'error' });
});

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.

import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest';
import { GadgetBackgroundInstanceForm } from './gadgetbackgroundinstanceform';
import React from 'react';
import { useGadgetConn } from '../gadgets/conn';

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.

useGadgetConn is imported but never used in this test file. This can fail TypeScript/ESLint checks (e.g., noUnusedLocals) and should be removed.

Suggested change
import { useGadgetConn } from '../gadgets/conn';

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