-
Notifications
You must be signed in to change notification settings - Fork 49
Increase viewport width #1870
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
base: master
Are you sure you want to change the base?
Increase viewport width #1870
Conversation
Reviewer's GuideEnhance Cypress test stability by setting the viewport width to 1300px, adding an authentication flag to the workspace test login, and switching users table selectors from OUIA IDs to accessible aria-labels in both tests and component code. Class diagram for updated DataViewTable usage in UsersTable and UserGroupsTableclassDiagram
class UsersTable {
- aria-label: string (changed from "Users Table" to "users table")
- ouiaId: string
- columns
- rows
}
class UserGroupsTable {
- aria-label: string (changed from "Users Table" to "users table")
- ouiaId: string
- columns
- rows
}
UsersTable --> DataViewTable
UserGroupsTable --> DataViewTable
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @karelhala - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it's still using that non-admin user and unable to find the users table
14:27:23
14:27:23 1) Make user an org admin
14:27:23 "before each" hook for "Grants org admin status to a user":
14:27:23 AssertionError: Timed out retrying after 100000ms: Expected to find element: `table[aria-label="users table"]`, but never found it.
14:27:23
14:27:23 Because this error occurred during a `before each` hook we are skipping the remaining tests in the current suite: `Make user an org admin`
14:27:23 at waitForUsersTable (webpack://insights-****/./cypress/e2e/user-access.cy.ts:5:65)
14:27:23 at navigateToUsersTable (webpack://insights-****/./cypress/e2e/user-access.cy.ts:10:0)
14:27:23 at Context.eval (webpack://insights-****/./cypress/e2e/user-access.cy.ts:58:0)
14:27:23
14:27:23 2) Activate/deactivate users from users table
14:27:23 Single user action via toggle
14:27:23 "before each" hook for "activates an inactive user":
14:27:23 AssertionError: Timed out retrying after 100000ms: Expected to find element: `table[aria-label="users table"]`, but never found it.```
7986d05
to
3a9f776
Compare
/retest |
3a9f776
to
3a39c1b
Compare
Description
When we are running tests cypress defaults to smaller screen, that breaks our flow since some elements are hidden or changed. This PR fixes such issue by setting direct width to 1300px. There's also flaky test in users table where it tries to access ouiaId which is not present on the screen that's because we are using users table at one test and in other we are using users and groups. This PR changes from ouia ID to aria-label so we are testing directly what user can see and interact with.
Summary by Sourcery
Improve Cypress test stability by expanding the viewport size and updating selectors for consistency with user-facing attributes.
Enhancements:
Tests: