Add tests for OTP tokens pages - #1149
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The OTP feature files repeat a lot of boilerplate setup/cleanup (same users/tokens created and deleted in many scenarios); consider extracting common Given steps into shared step definitions or using Background/Scenario Outlines to reduce duplication and make the flows easier to maintain.
- The updated selectOption helper now assumes that every option element contains a button and uses click({ force: true }); if possible, tighten the selector (e.g., a specific data-cy or role) and assert the button exists before clicking, and only fall back to a forced click when the normal click fails or when a flag is passed, to avoid masking real UI issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The OTP feature files repeat a lot of boilerplate setup/cleanup (same users/tokens created and deleted in many scenarios); consider extracting common Given steps into shared step definitions or using Background/Scenario Outlines to reduce duplication and make the flows easier to maintain.
- The updated selectOption helper now assumes that every option element contains a button and uses click({ force: true }); if possible, tighten the selector (e.g., a specific data-cy or role) and assert the button exists before clicking, and only fall back to a forced click when the normal click fails or when a flag is passed, to avoid masking real UI issues.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
39cc0d3 to
f8d00b9
Compare
5d689e4 to
49e37da
Compare
The tests should cover all the use cases of the 'OTP tokens' pages (main, 'Settings', and 'Is managed by'). Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Carla Martinez <carlmart@redhat.com>
49e37da to
7931251
Compare
|
@duzda - I think I amended all the changes mentioned in your feedback. |
veronnicka
left a comment
There was a problem hiding this comment.
Hi, I have a few suggestions, please look into them.
| Then I should see "otpuser" option in the "modal-select-owner" selector | ||
|
|
||
| When I click on the "modal-button-add" button | ||
| Then I should not see "add-otp-token-modal" modal |
There was a problem hiding this comment.
And I should see the token in the table or something along the lines
There was a problem hiding this comment.
similar to this
```
And I should see "add-managedby-success" alert
When I search for "otpmanager" in the members table
Then I should see "otpmanager" entry in the data table
| Then I should see "otpuser" option in the "modal-select-owner" selector | ||
|
|
||
| When I click on the "modal-button-add" button | ||
| Then I should not see "add-otp-token-modal" modal |
There was a problem hiding this comment.
And I should see the token in the table
| Then I should not see "add-otp-token-modal" modal | ||
|
|
||
| @test | ||
| Scenario: Delete button is disabled when no token is selected |
There was a problem hiding this comment.
I dont think this test is needed
There was a problem hiding this comment.
and the following two as well
| When I click on the "modal-button-ok" button | ||
| Then I should not see "enable-disable-otp-tokens-modal" modal | ||
| And I should see "success" alert | ||
|
|
There was a problem hiding this comment.
there should be @cleanup and @seed again between these tests
| And I should not see "delete-otp-tokens-modal" modal | ||
|
|
||
| @cleanup | ||
| Scenario: Cleanup: Delete test data and remaining tokens |
There was a problem hiding this comment.
nitpick: the name of the cleanup is a bit confusing
| And OTP token "settings_token" exists for user "otpsettingsuser" with type "totp" and description "settings_token" | ||
|
|
||
| @test | ||
| Scenario: Save and Revert buttons are disabled when no changes |
There was a problem hiding this comment.
I think this test is not necessary
|
|
||
| When I click on the "otp-tokens-tab-settings-button-save" button | ||
| Then I should see "success" alert | ||
| And I should see the "otp-tokens-tab-settings-button-save" button is disabled |
There was a problem hiding this comment.
should we check for these buttons? if yes, they are missing in the next test. I would suggest to remove the checks for the buttons
| And OTP token "settings_token" exists for user "otpsettingsuser" with type "totp" and description "settings_token" | ||
|
|
||
| @test | ||
| Scenario: Refresh button is available |
There was a problem hiding this comment.
Either remove this or put it with the test with other buttons
| And OTP token "settings_token" exists for user "otpsettingsuser" with type "totp" and description "settings_token" | ||
|
|
||
| @test | ||
| Scenario: Kebab menu opens and contains actions |
There was a problem hiding this comment.
is it really testing if it contains actions?
|
|
||
| When I click on the "modal-button-ok" button | ||
| Then I should not see "enable-disable-otp-tokens-modal" modal | ||
| And I should see "success" alert |
There was a problem hiding this comment.
Im missing a check if its really enabled/disabled.
There was a problem hiding this comment.
same for the enable test below
The tests should cover all the use cases
of the 'OTP tokens' pages (main, 'Settings',
and 'Is managed by').
Assisted-by: Claude noreply@anthropic.com
Summary by Sourcery
Add comprehensive end-to-end coverage for OTP token management, settings, and manager assignment flows, including supporting test helpers for IPA-backed OTP operations and UI interactions.
New Features:
Enhancements:
Tests: