Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use the two-steps log in on hosted #4399

Merged
merged 1 commit into from
May 20, 2024

Conversation

tranchitella
Copy link
Contributor

Changelog: None
Ticket: MEN-6823

@mender-test-bot
Copy link
Contributor

@tranchitella, Let me know if you want to start the integration pipeline by mentioning me and the command "start pipeline".


my commands and options

You can trigger a pipeline on multiple prs with:

  • mentioning me and start pipeline --pr mender/127 --pr mender-connect/255

You can start a fast pipeline, disabling full integration tests with:

  • mentioning me and start pipeline --fast

You can trigger GitHub->GitLab branch sync with:

  • mentioning me and sync

You can cherry pick to a given branch or branches with:

  • mentioning me and:
 cherry-pick to:
 * 1.0.x
 * 2.0.x

Comment on lines 48 to +53
await act(async () => {
await user.type(screen.getByLabelText(/your email/i), 'something-2fa@example.com');
});
await act(async () => {
await user.click(screen.getByRole('button', { name: /Log in/i }));
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await act(async () => {
await user.type(screen.getByLabelText(/your email/i), 'something-2fa@example.com');
});
await act(async () => {
await user.click(screen.getByRole('button', { name: /Log in/i }));
});
await act(async () => {
await user.type(screen.getByLabelText(/your email/i), 'something-2fa@example.com');
await user.click(screen.getByRole('button', { name: /Log in/i }));
});

the call to act can take a bit of time to tear down and cause weird problems in test execution, thus the RTL recommendation is to bundle them similar to an active phase of a user interacting with a page

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code fails with:

  ● Login Component › works as intended

    Unable to perform pointer interaction as the element has `pointer-events: none`:

    BUTTON(label=Log in)

      48 |     await act(async () => {
      49 |       await user.type(screen.getByLabelText(/your email/i), 'something-2fa@example.com');
    > 50 |       await user.click(screen.getByRole('button', { name: /Log in/i }));
         |       ^
      51 |     });
      52 |     expect(loginSpy).toHaveBeenCalled();
      53 |     await waitFor(() => rerender(ui));

I believe this is because we have to wait the renderer to update the button and activate it before being able to click it. Based on this assumption, it seems to me that two different act calls are needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the pipeline is passing and you approved the PR, @mzedel.
If I need to rework this particular section, let me know and I will follow up with a dedicated PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation! The recommendation would be to be explicit and do a
await waitFor(() => expect(screen.getByRole('button', { name: /log in/i })).not.toBeDisabled());
or often an
await waitFor(() => rerender(ui));
also works...

but OTOH - if it works I won't mind... and absolutely: once approved always merge ahead!

Changelog: None
Ticket: MEN-6823

Signed-off-by: Fabio Tranchitella <fabio.tranchitella@northern.tech>
@tranchitella tranchitella merged commit 6cadebb into mendersoftware:master May 20, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants