Group e2e tests based on users access level.#292
Open
maryam-sheikhi wants to merge 1 commit intomainfrom
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reorganizes the Playwright e2e tests by user access level (public/user/admin) and introduces shared fixtures to reduce duplication around login and admin navigation.
Changes:
- Added shared Playwright fixtures (e.g.,
loggedInPage) and admin-specific fixtures (adminPageviaadminMenuItemName). - Updated existing user/admin tests to use the new shared/admin fixtures and adjusted import paths accordingly.
- Added a new public-page e2e test for the study landing/details page.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/user-pages/account.spec.ts | Switches to shared fixtures and updates imports/button name. |
| tests/shared-features/fixtures.ts | Introduces a shared loggedInPage fixture for authenticated flows. |
| tests/public-pages/study.spec.ts | Adds a new public test validating the study details page. |
| tests/public-pages/home.spec.ts | Fixes expectTitle import path after test regrouping. |
| tests/public-pages/browse.spec.ts | Fixes expectTitle import path after test regrouping. |
| tests/admin-pages/upload-box-manager.spec.ts | Refactors to use admin fixtures and shared navigation setup. |
| tests/admin-pages/iva-manager.spec.ts | Refactors to use admin fixtures and shared navigation setup. |
| tests/admin-pages/admin-fixtures.ts | Adds admin-specific fixtures to navigate to selected admin sections. |
| tests/admin-pages/access-request-manager.spec.ts | Refactors to use admin fixtures and corrects unauthenticated navigation URL. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
tests/user-pages/account.spec.ts:13
- This locator assertion runs before any
page.goto(...), so it will always pass on the default blank page and doesn’t validate the logged-out state. Consider navigating to a real page first (e.g./) and asserting the expected UI for logged-out users there, or remove this check.
tests/admin-pages/upload-box-manager.spec.ts:21 - These assertions run before any navigation, so they’ll always pass on the initial blank page and don’t actually verify the unauthenticated state. Consider moving them after a
page.goto('/')(or after the protected route navigation/redirect) and asserting the expected logged-out UI there, or remove them to avoid misleading checks.
tests/admin-pages/iva-manager.spec.ts:21 - These assertions run before any navigation, so they’ll always pass on the initial blank page and don’t actually verify the unauthenticated state. Consider moving them after a
page.goto('/')(or after the protected route navigation/redirect) and asserting the expected logged-out UI there, or remove them to avoid misleading checks.
tests/admin-pages/access-request-manager.spec.ts:21 - These assertions run before any navigation, so they’ll always pass on the initial blank page and don’t actually verify the unauthenticated state. Consider moving them after a
page.goto('/')(or after the protected route navigation/redirect) and asserting the expected logged-out UI there, or remove them to avoid misleading checks.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
tests/user-pages/account.spec.ts:12
- The test asserts
Log inis absent before any navigation; on the defaultabout:blankpage this will always be true and doesn’t actually verify the logged-out state. Navigate first (e.g.,page.goto('/')) and then assert the logged-out UI (e.g.,Log invisible and/orAccount/admin menu absent) before hitting/account.
tests/shared-features/fixtures.ts:32 loggedInPagecreates a page viabrowser.newPage(), which bypasses Playwright Test’s configured context options (notablyuse.baseURL). That makes relative navigations likepage.goto('/')unreliable. Prefer creating the page from the testcontext(e.g.,context.newPage()) so baseURL and other settings are applied consistently.
tests/admin-pages/upload-box-manager.spec.ts:20- This test checks
Log inhas count 0 before any navigation; on the defaultabout:blankpage this will always pass and doesn’t prove the user is logged out. Navigate to a real page first (e.g.,/) and then assert the logged-out state (e.g.,Log invisible, admin menu absent) before trying/upload-box-manager.
tests/admin-pages/iva-manager.spec.ts:20 - This test asserts
Log inhas count 0 before any navigation; on the initialabout:blankpage this will always pass and doesn’t validate the logged-out state. Navigate first (e.g.,page.goto('/')) and then assert the appropriate logged-out UI before visiting/iva-manager.
tests/admin-pages/access-request-manager.spec.ts:20 - This test asserts
Log inhas count 0 before any navigation; on the initialabout:blankpage this will always pass and doesn’t validate the logged-out state. Navigate first (e.g.,page.goto('/')) and then assert the appropriate logged-out UI before visiting/access-request-manager.
fa18d85 to
07800c9
Compare
Refactor admin fixture. Check menuItem Url in admin fixture. Remove duplicate URL checks. Refactor admin tests to use new fixtures and utilities.
07800c9 to
85b3843
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.