test: add tests for GenericGadgetRenderer component by mrhapile · Pull Request #57 · inspektor-gadget/headlamp-plugin

mrhapile · 2026-03-07T21:53:56Z

Add tests for GenericGadgetRenderer component

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

The tests verify that the component renders correctly and properly handles gadget rendering logic and fallback behavior when a gadget is not provided.

How to use

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

Steps:

Checkout this branch.
Install dependencies if necessary.
Run the test suite.

Testing done

Commands executed:

npm install
npm test

Result:

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

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

Copilot

Pull request overview

Adds an RTL/Vitest test suite for GenericGadgetRenderer to increase coverage of its side-effect-driven gadget lifecycle behavior (run vs attach, cleanup, and connection-related state updates).

Changes:

Added index.test.tsx covering runGadget execution, attachGadgetInstance delayed attachment, and stop/cleanup behavior across rerenders and unmount.
Added coverage for connection-driven setPodStreamsConnected updates and imageName decoding/handling.

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

Copilot · 2026-03-07T21:58:31Z

+        vi.useRealTimers();
+        cleanup();


In afterEach, vi.useRealTimers() runs before cleanup(). Since the component under test uses setTimeout, it’s safer to unmount/cleanup while fake timers are still enabled (so the component’s clearTimeout calls match the same timer implementation), then switch back to real timers to reduce risk of timer leaks/flakiness.

Suggested change

vi.useRealTimers();

cleanup();

cleanup();

vi.useRealTimers();

Copilot · 2026-03-07T21:58:31Z

+    test('timeout callback bails out if ig is falsy when running', () => {
+        const p = { ...defaultProps, gadgetInstance: { id: 'inst1', gadgetConfig: { version: 2 } } };
+        const { rerender } = render(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} />);
+
+        mockUsePortForward.mockReturnValue({ ig: undefined, isConnected: true });
+        // Trigger a re-render. Since we didn't change props used in useEffect deps, we'd need to change podStreamsConnected
+        rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} podStreamsConnected={2} podsSelected={['pod1', 'pod2']} />);
+        // But the timeout is already registered with the old ig variable state? No, `ig` is from usePortForward and available in the closure for timeout?
+        // Wait, the closure captures the initial `ig` value... 
+
+        // Let's completely unmount and remount with ig undefined
+        cleanup();
+        mockUsePortForward.mockReturnValue({ ig: undefined, isConnected: true });
+        render(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} />);
+
+        act(() => {
+            vi.advanceTimersByTime(2000);
+        });
+
+        expect(mockAttachGadgetInstance).not.toHaveBeenCalled();
+    });
+


The test "timeout callback bails out if ig is falsy when running" doesn’t actually validate the in-timeout !ig guard: after remounting with ig undefined, gadgetStartStopHandler returns early and no timeout is scheduled, so the assertion will pass even if the timeout guard is removed. Consider rewriting to directly invoke the scheduled timeout callback (similar to the unmount test) or drop this test to avoid a false sense of coverage.

Suggested change

test('timeout callback bails out if ig is falsy when running', () => {

const p = { ...defaultProps, gadgetInstance: { id: 'inst1', gadgetConfig: { version: 2 } } };

const { rerender } = render(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} />);

mockUsePortForward.mockReturnValue({ ig: undefined, isConnected: true });

// Trigger a re-render. Since we didn't change props used in useEffect deps, we'd need to change podStreamsConnected

rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} podStreamsConnected={2} podsSelected={['pod1', 'pod2']} />);

// But the timeout is already registered with the old ig variable state? No, `ig` is from usePortForward and available in the closure for timeout?

// Wait, the closure captures the initial `ig` value...

// Let's completely unmount and remount with ig undefined

cleanup();

mockUsePortForward.mockReturnValue({ ig: undefined, isConnected: true });

render(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} />);

act(() => {

vi.advanceTimersByTime(2000);

});

expect(mockAttachGadgetInstance).not.toHaveBeenCalled();

});

Copilot · 2026-03-07T21:58:31Z

+        // Trigger a re-render. Since we didn't change props used in useEffect deps, we'd need to change podStreamsConnected
+        rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} podStreamsConnected={2} podsSelected={['pod1', 'pod2']} />);
+        // But the timeout is already registered with the old ig variable state? No, `ig` is from usePortForward and available in the closure for timeout?
+        // Wait, the closure captures the initial `ig` value... 
+
+        // Let's completely unmount and remount with ig undefined


There are several large “thinking out loud” comments in the test body (e.g., questioning closure capture around the timeout). These make the test harder to read/maintain and can quickly become outdated. Prefer removing the commentary and keeping only the minimal explanation of what behavior is being asserted.

Suggested change

// Trigger a re-render. Since we didn't change props used in useEffect deps, we'd need to change podStreamsConnected

rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} podStreamsConnected={2} podsSelected={['pod1', 'pod2']} />);

// But the timeout is already registered with the old ig variable state? No, `ig` is from usePortForward and available in the closure for timeout?

// Wait, the closure captures the initial `ig` value...

// Let's completely unmount and remount with ig undefined

// Trigger a re-render by changing podStreamsConnected so the effect runs again.

rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} podStreamsConnected={2} podsSelected={['pod1', 'pod2']} />);

// Also verify behavior when the component is unmounted and remounted with ig undefined.

Copilot · 2026-03-07T21:58:32Z

+    let mockIg: any;
+    let mockRunGadget: any;
+    let mockAttachGadgetInstance: any;
+    let mockStopGadget: any;
+    let mockStopAttach: any;
+    let mockUsePortForward: any;
+
+    const defaultProps: any = {
+        podsSelected: ['pod1'],
+        podStreamsConnected: 1,
+        podSelected: 'pod1',
+        setGadgetConfig: vi.fn(),
+        dataColumns: { col1: [] },
+        gadgetRunningStatus: true,


The test file uses any for most mocks/props (defaultProps: any, mockIg: any, etc.), which bypasses TypeScript checks and can let incorrect test setups compile. Consider typing defaultProps as React.ComponentProps<typeof GenericGadgetRenderer> and giving mockIg/mocks minimal structural types so the tests fail at compile time when the component API changes.

Copilot · 2026-03-07T21:58:32Z

+        // Trigger StartStopHandler again by simulating podStreamsConnected change
+        rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} podStreamsConnected={1} />); // It actually runs again only if gadgetRunningStatus && length === connected and deps change..
+        // Wait, the dependencies are [gadgetRunningStatus, podStreamsConnected, podsSelected]
+        // Setting podStreamsConnected to a different value won't trigger it if it doesn't match length. 
+        // Setting podsSelected length might.
+        // Let's just go running false then true
+        rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={false} />);
+        rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} />);


This test includes several inline notes about how to trigger the effect (e.g., “Wait, the dependencies are … Let's just go running false then true”). These notes are confusing in committed test code and can be replaced by directly triggering the intended dependency change (e.g., change podsSelected/podStreamsConnected to matching values) with a short, stable comment.

Suggested change

// Trigger StartStopHandler again by simulating podStreamsConnected change

rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} podStreamsConnected={1} />); // It actually runs again only if gadgetRunningStatus && length === connected and deps change..

// Wait, the dependencies are [gadgetRunningStatus, podStreamsConnected, podsSelected]

// Setting podStreamsConnected to a different value won't trigger it if it doesn't match length.

// Setting podsSelected length might.

// Let's just go running false then true

rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={false} />);

rerender(<GenericGadgetRenderer {...p} gadgetRunningStatus={true} />);

// Trigger StartStopHandler again by updating podsSelected and podStreamsConnected to matching values

rerender(

<GenericGadgetRenderer

{...p}

gadgetRunningStatus={true}

podStreamsConnected={1}

podsSelected={['pod1']}

/>,

);

Copilot · 2026-03-07T21:58:32Z

+
+        render(<GenericGadgetRenderer {...defaultProps} gadgetRunningStatus={true} />);
+
+        errCb('test error');


In the "runGadget err callback" test, errCb is invoked with a string, but the production callback signature is onSetupError: (error: Error) => void. Passing an Error instance here would better reflect real usage and catch formatting/handling differences (e.g., logging error.message).

Suggested change

errCb('test error');

errCb(new Error('test error'));

Copilot

Pull request overview

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

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

Copilot · 2026-04-07T07:55:03Z

+        mockUsePortForward.mockReturnValue({ ig: mockIg, isConnected: true });
+
+        (createGadgetCallbacks as any).mockReturnValue({
+            onData: vi.fn()


The createGadgetCallbacks mock only returns onData, but GenericGadgetRenderer passes the callbacks object to ig.runGadget/ig.attachGadgetInstance and also spreads it when overriding onReady. To keep the test resilient to future changes (and closer to the real callback contract), return no-op implementations for the full callback shape (onReady, onDone, onError, onGadgetInfo, etc.) instead of a partial object.

Suggested change

onData: vi.fn()

onData: vi.fn(),

onReady: vi.fn(),

onDone: vi.fn(),

onError: vi.fn(),

onGadgetInfo: vi.fn(),

Copilot · 2026-04-07T07:55:04Z

+        errCb('test error');
+        expect(consoleSpy).toHaveBeenCalledWith('Gadget run error:', 'test error');


runGadget's setup error callback is typed as (error: Error) => void (see IGConnection), but this test invokes it with a string. Passing an Error instance here (and asserting on it) will better reflect real usage and prevent accidental type/shape regressions.

Suggested change

errCb('test error');

expect(consoleSpy).toHaveBeenCalledWith('Gadget run error:', 'test error');

const error = new Error('test error');

errCb(error);

expect(consoleSpy).toHaveBeenCalledWith('Gadget run error:', error);

illume · 2026-04-07T08:20:10Z

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

test: add tests for GenericGadgetRenderer component

d41bea8

Signed-off-by: mrhapile <allinonegaming3456@gmail.com>

Copilot AI review requested due to automatic review settings March 7, 2026 21:53

Copilot started reviewing on behalf of mrhapile March 7, 2026 21:54 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

illume requested a review from Copilot April 7, 2026 07:51

Copilot started reviewing on behalf of illume April 7, 2026 07:52 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

test: add tests for GenericGadgetRenderer component - #57

test: add tests for GenericGadgetRenderer component#57
mrhapile wants to merge 1 commit into
inspektor-gadget:mainfrom
mrhapile:test/generic-gadget-renderer

mrhapile commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

illume commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		render(<GenericGadgetRenderer {...defaultProps} gadgetRunningStatus={true} />);

		errCb('test error');

-            onData: vi.fn()
+            onData: vi.fn(),
+            onReady: vi.fn(),
+            onDone: vi.fn(),
+            onError: vi.fn(),
+            onGadgetInfo: vi.fn(),

		errCb('test error');
		expect(consoleSpy).toHaveBeenCalledWith('Gadget run error:', 'test error');

-        errCb('test error');
-        expect(consoleSpy).toHaveBeenCalledWith('Gadget run error:', 'test error');
+        const error = new Error('test error');
+        errCb(error);
+        expect(consoleSpy).toHaveBeenCalledWith('Gadget run error:', error);

Uh oh!

Conversation

mrhapile commented Mar 7, 2026

Add tests for GenericGadgetRenderer component

How to use

Testing done

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

illume commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants