Skip to content

Fixes #38714 - use common LabelIcon for popup help#1000

Merged
MariaAga merged 1 commit intotheforeman:masterfrom
kfamilonidis:fix-help-button
Sep 25, 2025
Merged

Fixes #38714 - use common LabelIcon for popup help#1000
MariaAga merged 1 commit intotheforeman:masterfrom
kfamilonidis:fix-help-button

Conversation

@kfamilonidis
Copy link
Copy Markdown
Contributor

The info button after Hosts -> Create Host -> Advanced -> "Remote Execution Interface" looks ok.

Copy link
Copy Markdown
Member

@MariaAga MariaAga left a comment

Choose a reason for hiding this comment

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

Works well, a few optional requests:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since tests are run from foreman core directly, using npm run test:plugins foreman_remote_execution its not necessary to mock foreman components any more

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed, there is always the need to mock the component though with jest, not sure i there is a better way yet..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is there always a need to mock a component through jest?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the module cannot be resolved, looking at the jest config maybe is missing some entry (is absent)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When removing this file I only saw:

 TestingLibraryElementError: Unable to find an element with the text: Identifier of the Host interface for Remote execution. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

Which is expected since its a button with a tooltip, that only opens on click, and the test didnt click it.
you can amend the test to be:

  it('should render label with help icon and popover instructions', async () => {
    jest.useFakeTimers();
    render(<RexInterface {...fixtures.renders} />);
    await act(async () => {
      await fireEvent.click(screen.getByRole('button'));

      jest.advanceTimersByTime(1000);
    });

And then it passes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok. Thanks.


const fixtures = {
renders: { isLoading: false, onChange: () => {} },
renders: {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since its a small component, do you mind also updating the test from snapshot test to React Testing library test? (instead of updating the snapshot)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@kfamilonidis kfamilonidis force-pushed the fix-help-button branch 5 times, most recently from caff037 to 0e53773 Compare September 23, 2025 10:23
Refactors the popup help icon to use the shared LabelIcon component.
The Enzyme tests for this component have been replaced with new tests
using React Testing Library to align with our current testing standards

Fixes ddc986e (Fixes #32218 - Host Registration - New form)
Copy link
Copy Markdown
Member

@MariaAga MariaAga left a comment

Choose a reason for hiding this comment

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

Thanks!

@MariaAga MariaAga merged commit 2b495c9 into theforeman:master Sep 25, 2025
25 checks passed
@kfamilonidis kfamilonidis deleted the fix-help-button branch September 25, 2025 12:07
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.

2 participants