chore: refactor cypress tests #1060
Conversation
Strift
added
the
maintenance
WalkthroughAdds cursor rule configs; updates .gitignore and playground ignores; replaces/adjusts Cypress test suites and support utilities; updates scripts, Cypress dependency, CI workflows and cypress.config; tweaks playground admin/schema; replaces playground register with an async bootstrap that seeds or updates an admin user. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer / CI
participant Playground as Playground App
participant AdminSvc as admin::user service
participant RoleSvc as role service
Dev->>Playground: start bootstrap({ strapi })
Playground->>AdminSvc: findOne({ email })
alt admin not found
Playground->>RoleSvc: findOne({ code: "strapi-super-admin" })
RoleSvc-->>Playground: role
Playground->>AdminSvc: create({ email, password, roles: [role.id] })
AdminSvc-->>Playground: user created
else admin exists
Playground->>AdminSvc: if password == 'password' -> update(userId, { password })
AdminSvc-->>Playground: updated (optional)
end
Playground-->>Dev: log success / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (9)
cypress/support/commands.js (1)
38-38: Consider the impact of removing the timeout option.The timeout option was removed from
cy.get('form'). While the current implementation usesshould('be.visible')which has its own timeout, ensure this change doesn't affect tests running against slower environments.cypress/e2e/permissions.cy.js (4)
105-113: Scope selection with find; avoid nested getget(...) resets the subject to document. Use find to scope to the nav and streamline the click flow.
- cy.get('nav') - .get('a[aria-label="Meilisearch"]', { timeout: 10000 }) - .should('be.visible') - cy.get('nav') - .get('a[aria-label="Meilisearch"]', { timeout: 10000 }) - .click() + cy.get('nav') + .find('a[aria-label="Meilisearch"]', { timeout: 10000 }) + .should('be.visible') + .click()
156-161: Prefer cy.contains/find over jQuery :contains inside cy.getSelectors like tr:contains("user") inside get are brittle. Use cy.contains to locate the row, then scope with find.
- cy.get('tr:contains("user") button[role="checkbox"]').first().click() + cy.contains('tr', 'user').find('button[role="checkbox"]').first().click() @@ - cy.get('tr:contains("user")').contains('Yes').should('be.visible') - cy.get('tr:contains("user")').contains('1 / 1').should('be.visible') - cy.get('tr:contains("user")').contains('Hooked').should('be.visible') + cy.contains('tr', 'user').should('contain.text', 'Yes') + cy.contains('tr', 'user').should('contain.text', '1 / 1') + cy.contains('tr', 'user').should('contain.text', 'Hooked') @@ - cy.get('tr:contains(user)').first().contains('Yes').should('be.visible') - cy.get('tr:contains(user)') - .first() - .contains('Hooked') - .should('be.visible') - cy.get('tr:contains(user)').first().contains('button', 'Update').click() + cy.contains('tr', 'user').should('contain.text', 'Yes') + cy.contains('tr', 'user').should('contain.text', 'Hooked') + cy.contains('tr', 'user').contains('button', 'Update').click() @@ - cy.get('tr:contains(user)').first().contains('Yes').should('be.visible') - cy.get('tr:contains(user)') - .first() - .contains('Hooked') - .should('be.visible') - cy.get('tr:contains(user)') - .first() - .get('button[role="checkbox"]') - .first() - .click() + cy.contains('tr', 'user').should('contain.text', 'Yes') + cy.contains('tr', 'user').should('contain.text', 'Hooked') + cy.contains('tr', 'user').find('button[role="checkbox"]').first().click() @@ - cy.get('tr:contains(user)') - .first() - .contains('Reload needed') - .should('be.visible') + cy.contains('tr', 'user').contains('Reload needed').should('be.visible')Also applies to: 166-177, 223-232, 271-287
186-187: Standardize negative assertions for buttonsReplace "root().should('not.contain', 'button:contains(...)')" with direct non-existence checks.
- cy.root().should('not.contain', 'button:contains("Update")') + cy.contains('button', 'Update').should('not.exist') @@ - cy.root().should('not.contain', 'button:contains("Save")') + cy.contains('button', 'Save').should('not.exist')Apply similarly at the other cited locations.
Also applies to: 192-193, 236-237, 265-266, 335-336, 340-341
35-43: Extract helpers to Cypress commandsloginUser and visitPluginPage are duplicated across suites. Move them into cypress/support/commands.js (or a shared util) for reuse and stability.
I can provide a commands.js snippet if helpful.
Also applies to: 46-50
cypress/e2e/features.cy.js (3)
69-70: Use cy.contains for text instead of jQuery :contains inside getPrefer cy.contains('button', 'Settings') over cy.get('button:contains("Settings")') for clarity and reliability.
- cy.get('button:contains("Settings")').click() + cy.contains('button', 'Settings').click() @@ - cy.get('button:contains("Settings")').click() + cy.contains('button', 'Settings').click()Apply similarly at the other cited locations.
Also applies to: 86-87, 94-98, 111-118, 135-140
35-44: Use forEach for side-effects (avoid map)map is unused; forEach is clearer for iterative assertions.
- const checkCollectionContent = ({ rowNb, contains }) => { + const checkCollectionContent = ({ rowNb, contains }) => { const row = `table[role='grid'] tbody tr:nth-child(${rowNb})` - contains.map(value => + contains.forEach(value => cy .get(row, { timeout: 10000, }) .contains(value, { timeout: 10000 }), - ) + ) }
46-64: Avoid static waits in cy.session validateRely on content-based waits only. Drop cy.wait(1000) to reduce flakiness.
- cy.wait(1000) cy.contains('Hello User who can manage Meilisearch').should( 'be.visible', )playground/package.json (1)
7-7: Ensure target dir exists and overwrite deterministicallyPlain cp will fail if .tmp is missing. Create the directory and overwrite.
- "setup": "cp pre-seeded-database.db .tmp/data.db", + "setup": "mkdir -p .tmp && cp -f pre-seeded-database.db .tmp/data.db",If Windows support is required, consider using shx: "setup": "shx mkdir -p .tmp && shx cp -f pre-seeded-database.db .tmp/data.db".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
playground/.tmp/data.dbis excluded by!**/*.dbplayground/pre-seeded-database.dbis excluded by!**/*.dbplayground/types/generated/contentTypes.d.tsis excluded by!**/generated/**playground/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
.cursor/rules/cypress.mdc(1 hunks).cursor/rules/general.mdc(1 hunks).gitignore(2 hunks)cypress/e2e/admin.cy.js(0 hunks)cypress/e2e/features.cy.js(1 hunks)cypress/e2e/permissions.cy.js(1 hunks)cypress/e2e/roles.cy.js(0 hunks)cypress/e2e/utils.js(1 hunks)cypress/support/commands.js(1 hunks)package.json(2 hunks)playground/.gitignore(1 hunks)playground/config/admin.js(1 hunks)playground/package.json(1 hunks)playground/src/api/restaurant/content-types/restaurant/schema.json(1 hunks)playground/src/index.js(1 hunks)
💤 Files with no reviewable changes (2)
- cypress/e2e/admin.cy.js
- cypress/e2e/roles.cy.js
🧰 Additional context used
🪛 Biome (2.1.2)
cypress/e2e/utils.js
[error] 1-6: Illegal use of an export declaration outside of a module
not allowed inside scripts
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cypress-run
🔇 Additional comments (11)
.cursor/rules/cypress.mdc (1)
1-6: LGTM!The Cursor rule configuration is well-structured and provides helpful context for developers working with Cypress tests.
package.json (1)
10-11: Good separation of setup and dev scripts.The new
playground:setupscript and the simplifiedplayground:devscript improve the workflow clarity and align with the seeding strategy refactor..cursor/rules/general.mdc (1)
1-7: LGTM!The general rule configuration appropriately documents the project's package manager conventions.
playground/src/api/restaurant/content-types/restaurant/schema.json (1)
7-7: LGTM!The capitalization of the display name improves the UI presentation and follows proper naming conventions.
.gitignore (2)
75-75: LGTM!Enabling the
.tmpdirectory ignore follows standard practices for temporary files.
136-139: LGTM!The addition of
distto the ignore list and the exception forplayground/.tmp/data.dbproperly support the new seeding strategy. The exception ensures the pre-seeded database remains tracked for the playground setup workflow.playground/.gitignore (1)
134-134: LGTM!The trailing newline ensures proper file formatting.
cypress/support/commands.js (1)
33-51: Nocy.login(calls found. Manually verify any login invocations pass the requiredshouldContainargument.cypress/e2e/features.cy.js (1)
141-165: Sync after toggles: rely on intercepted aliases for each rowGood use of intercepts. Minor: ensure waits pair to each click to avoid race across loop iterations.
Consider moving waits inside the click branch (already done) and adding a short assertion per row after @fetchCollections completes, as you did below. LGTM otherwise.
playground/config/admin.js (1)
2-2: autoOpen: false is appropriate for CI/test runsPrevents auto-opening the admin UI during setup. LGTM.
Confirm this property is supported in your Strapi version (5.9.0). If needed, we can quickly check docs.
playground/src/index.js (1)
7-17: Use Strapi v5 APIs for queries and updates
- Replace
strapi.query('admin::user').findOnewithstrapi.db.query('admin::user').findOne- Replace
adminUserService.updateById(adminUser.id, {...})with the v5 update signature, e.g.:await strapi.db.query('admin::user').update({ where: { id: adminUser.id }, data: { password: 'password' }, });
| cy.get('div[role="status').contains('success').should('be.visible') | ||
| cy.get('div[role="status') | ||
| .contains('Credentials successfully updated') | ||
| .should('be.visible') | ||
| cy.removeNotifications() |
There was a problem hiding this comment.
Invalid attribute selector; fix bracket/quote
Selector 'div[role="status' is malformed; tests will fail to locate the status element.
- cy.get('div[role="status').contains('success').should('be.visible')
- cy.get('div[role="status')
+ cy.get('div[role="status"]').contains('success').should('be.visible')
+ cy.get('div[role="status"]')
.contains('Credentials successfully updated')
.should('be.visible')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cy.get('div[role="status').contains('success').should('be.visible') | |
| cy.get('div[role="status') | |
| .contains('Credentials successfully updated') | |
| .should('be.visible') | |
| cy.removeNotifications() | |
| cy.get('div[role="status"]').contains('success').should('be.visible') | |
| cy.get('div[role="status"]') | |
| .contains('Credentials successfully updated') | |
| .should('be.visible') | |
| cy.removeNotifications() |
🤖 Prompt for AI Agents
In cypress/e2e/features.cy.js around lines 77 to 81 the attribute selector is
malformed ('div[role="status' missing the closing quote and bracket), causing
the test to fail to find the status element; update both selectors to use the
correct syntax div[role="status"] (e.g.,
cy.get('div[role="status"]').contains('success').should('be.visible') and
cy.get('div[role="status"]').contains('Credentials successfully
updated').should('be.visible') so the attribute selector is properly closed.
| cy.get('input[name="title').type('The slimy snail') | ||
| cy.get('form').contains('button', 'Save').click() | ||
|
|
There was a problem hiding this comment.
Fix selector for title input
Missing closing bracket in attribute selector.
- cy.get('input[name="title').type('The slimy snail')
+ cy.get('input[name="title"]').type('The slimy snail')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cy.get('input[name="title').type('The slimy snail') | |
| cy.get('form').contains('button', 'Save').click() | |
| cy.get('input[name="title"]').type('The slimy snail') | |
| cy.get('form').contains('button', 'Save').click() |
🤖 Prompt for AI Agents
In cypress/e2e/features.cy.js around lines 265 to 267 the attribute selector for
the title input is missing a closing quote/bracket: update the selector to
correctly target the input element (e.g., input[name="title"] ), then run the
test to ensure the .type('The slimy snail') call targets the field before
clicking the Save button.
| it('cannot see the plugin in the sidepanel', () => { | ||
| cy.visit(`${adminUrl}`) | ||
| cy.get('nav').should('not.contain', 'a[aria-label="Meilisearch"]') | ||
| }) | ||
|
|
There was a problem hiding this comment.
Fix negative existence assertion in sidepanel check
Using "not.contain" with a selector string checks text, not element presence. Assert element non-existence instead.
Apply this diff:
- cy.visit(`${adminUrl}`)
- cy.get('nav').should('not.contain', 'a[aria-label="Meilisearch"]')
+ cy.visit(`${adminUrl}`)
+ cy.get('nav').find('a[aria-label="Meilisearch"]').should('not.exist')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('cannot see the plugin in the sidepanel', () => { | |
| cy.visit(`${adminUrl}`) | |
| cy.get('nav').should('not.contain', 'a[aria-label="Meilisearch"]') | |
| }) | |
| it('cannot see the plugin in the sidepanel', () => { | |
| cy.visit(`${adminUrl}`) | |
| cy.get('nav').find('a[aria-label="Meilisearch"]').should('not.exist') | |
| }) |
🤖 Prompt for AI Agents
In cypress/e2e/permissions.cy.js around lines 71 to 75, the test uses
cy.get('nav').should('not.contain', 'a[aria-label="Meilisearch"]'), which checks
text content rather than element presence; change it to assert element
non-existence by querying the link selector inside the nav and asserting it does
not exist (for example, use nav.find('a[aria-label="Meilisearch"]') and assert
.should('not.exist') or use cy.get('nav').within(...) and assert the selector
does not exist).
| it('cannot create an index', () => { | ||
| visitPluginPage() | ||
| // There are checkboxes in front of each collection | ||
| cy.root().should('not.contain', 'button[role="checkbox"]') | ||
| }) | ||
|
|
||
| it('cannot change settings', () => { | ||
| visitPluginPage() | ||
| cy.root().should('not.contain', 'button:contains("Save")') | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Use existence checks, not text-based "not.contain"
For "cannot create index" and "cannot change settings", assert element absence directly.
- // There are checkboxes in front of each collection
- cy.root().should('not.contain', 'button[role="checkbox"]')
+ // There are checkboxes in front of each collection
+ cy.get('button[role="checkbox"]').should('not.exist')
@@
- cy.root().should('not.contain', 'button:contains("Save")')
+ cy.contains('button', 'Save').should('not.exist')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('cannot create an index', () => { | |
| visitPluginPage() | |
| // There are checkboxes in front of each collection | |
| cy.root().should('not.contain', 'button[role="checkbox"]') | |
| }) | |
| it('cannot change settings', () => { | |
| visitPluginPage() | |
| cy.root().should('not.contain', 'button:contains("Save")') | |
| }) | |
| }) | |
| it('cannot create an index', () => { | |
| visitPluginPage() | |
| // There are checkboxes in front of each collection | |
| cy.get('button[role="checkbox"]').should('not.exist') | |
| }) | |
| it('cannot change settings', () => { | |
| visitPluginPage() | |
| cy.contains('button', 'Save').should('not.exist') | |
| }) | |
| }) |
🤖 Prompt for AI Agents
In cypress/e2e/permissions.cy.js around lines 115 to 125, the tests use
text-based "not.contain" assertions to check absence; replace them with direct
DOM existence checks: for the "cannot create an index" test assert that the
checkbox button selector (button[role="checkbox"]) does not exist, and for the
"cannot change settings" test assert that the Save button (use a selector such
as a button containing the text "Save" or a specific save button selector) does
not exist; use Cypress existence assertions (e.g., .should('not.exist')) to
perform these checks.
| cy.get('button[role="checkbox"]').first().should('not.be.checked') | ||
| }) |
There was a problem hiding this comment.
"be.checked" is invalid for button role checkboxes
The element is a button, not an input. Assert ARIA/data-state instead.
- cy.get('button[role="checkbox"]').first().should('not.be.checked')
+ cy.get('button[role="checkbox"]')
+ .first()
+ .should($btn => {
+ const state = $btn.attr('aria-checked') || $btn.attr('data-state')
+ expect(state).to.not.eq('true')
+ expect(state).to.not.eq('checked')
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cy.get('button[role="checkbox"]').first().should('not.be.checked') | |
| }) | |
| cy.get('button[role="checkbox"]') | |
| .first() | |
| .should($btn => { | |
| const state = $btn.attr('aria-checked') || $btn.attr('data-state') | |
| expect(state).to.not.eq('true') | |
| expect(state).to.not.eq('checked') | |
| }) | |
| }) |
🤖 Prompt for AI Agents
In cypress/e2e/permissions.cy.js around lines 304-305, the test uses
.should('not.be.checked') on a button with role="checkbox" which is invalid for
non-input elements; replace that assertion with an ARIA/data-state check — e.g.,
assert the element's aria-checked attribute equals "false"
(cy.get(...).first().should('have.attr', 'aria-checked', 'false')) or, if your
implementation uses data-state, assert the appropriate data-state value (e.g.,
'unchecked').
| export const STRAPI_ADMIN_ROLES = { | ||
| SUPER_ADMIN: 1, | ||
| ADMIN: 2, | ||
| EDITOR: 3, | ||
| AUTHOR: 4, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Centralized role constants are good, but verify ES module configuration.
The STRAPI_ADMIN_ROLES constant provides a clean centralized reference for role IDs across tests.
However, the static analysis error suggests this file may not be properly configured as an ES module. Ensure Cypress is configured to support ES modules (typically via cypress.config.js with specPattern and proper module settings).
🏁 Script executed:
#!/bin/bash
# Verify Cypress configuration supports ES modules
cat cypress.config.js 2>/dev/null || cat cypress.config.ts 2>/dev/null || echo "Cypress config not found"Length of output: 836
Use CommonJS exports in cypress/e2e/utils.js
Cypress is configured as CommonJS, so replace the ES module export with:
-export const STRAPI_ADMIN_ROLES = {
+module.exports.STRAPI_ADMIN_ROLES = {
SUPER_ADMIN: 1,
ADMIN: 2,
EDITOR: 3,
AUTHOR: 4,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const STRAPI_ADMIN_ROLES = { | |
| SUPER_ADMIN: 1, | |
| ADMIN: 2, | |
| EDITOR: 3, | |
| AUTHOR: 4, | |
| } | |
| module.exports.STRAPI_ADMIN_ROLES = { | |
| SUPER_ADMIN: 1, | |
| ADMIN: 2, | |
| EDITOR: 3, | |
| AUTHOR: 4, | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-6: Illegal use of an export declaration outside of a module
not allowed inside scripts
(parse)
🤖 Prompt for AI Agents
In cypress/e2e/utils.js lines 1-6, the file currently uses an ES module export;
change it to CommonJS by declaring the constant and exporting it with
module.exports. Replace the "export const STRAPI_ADMIN_ROLES = { ... }" syntax
with a plain const declaration and add "module.exports = { STRAPI_ADMIN_ROLES }"
so Cypress can require the value under CommonJS.
| * @param {string} password | ||
| */ | ||
| const login = ({ adminUrl, email, password }) => { | ||
| const login = ({ adminUrl, email, password, shouldContain }) => { |
There was a problem hiding this comment.
Add a default value or validation for the shouldContain parameter.
The new shouldContain parameter is not optional but lacks a default value. If callers don't provide this parameter, Line 47 will fail with undefined being passed to cy.contains().
Apply this diff to provide a sensible default:
-const login = ({ adminUrl, email, password, shouldContain }) => {
+const login = ({ adminUrl, email, password, shouldContain = 'Welcome' }) => {Or add validation:
const login = ({ adminUrl, email, password, shouldContain }) => {
+ if (!shouldContain) {
+ throw new Error('shouldContain parameter is required for login validation')
+ }
cy.session(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const login = ({ adminUrl, email, password, shouldContain }) => { | |
| const login = ({ adminUrl, email, password, shouldContain = 'Welcome' }) => { |
🤖 Prompt for AI Agents
In cypress/support/commands.js around line 33, the login function signature
currently declares shouldContain without a default which can pass undefined into
cy.contains(); update the function to either give shouldContain a safe default
(e.g. shouldContain = undefined) and only call cy.contains(shouldContain) when
it's not undefined, or add an explicit validation at the start of the function
that checks if shouldContain is provided before invoking cy.contains; ensure
callers behavior is unchanged when shouldContain is supplied.
| "babel-jest": "^29.7.0", | ||
| "concurrently": "^8.2.2", | ||
| "cypress": "^14.0.1", | ||
| "cypress": "^15.3.0", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify Cypress v15 compatibility.
The major version bump from Cypress ^14.0.1 to ^15.3.0 may introduce breaking changes. Ensure all existing tests remain compatible with Cypress v15.
🌐 Web query:
What are the breaking changes in Cypress v15 compared to v14?
💡 Result:
Short list of breaking changes introduced in Cypress v15 (changes you must address when moving from v14 → v15):
- Node.js support: Node 18 and 23 are no longer supported; supported installers require Node 20, 22, 24+. [1][2]
- Linux/glibc: Binaries no longer support Linux distributions with glibc older than 2.31. [1][2]
- Firefox automation: Chrome DevTools Protocol support for Firefox removed. [1]
- cy.exec result property rename: the returned property code → exitCode (update any .its('code') usages). [2]
- cy.stub signature: removed support for the deprecated three-argument signature of cy.stub. [1]
- API rename: Cypress.SelectorPlayground renamed to Cypress.ElementSelector (update any references). [3]
For migration details and examples see the official v15 changelog and migration guide. [1][2]
Sources: Cypress v15 changelog, v15 migration guide, Cypress blog post. [1][2][3]
Update tests for Cypress v15 breaking changes
- Ensure CI runs on Node 20/22/24+ (Node 18 & 23 unsupported)
- Verify Linux images use glibc ≥2.31
- Remove Firefox CDP automation usage
- Replace all
cy.exec().its('code')→.its('exitCode') - Remove any 3-argument
cy.stub()calls - Rename
Cypress.SelectorPlayground→Cypress.ElementSelector
| // Update the password if it's not hashed (for existing users) | ||
| if (adminUser.password === 'password') { | ||
| const adminUserService = strapi.service('admin::user'); | ||
| await adminUserService.updateById(adminUser.id, { | ||
| password: 'password' | ||
| }); | ||
| console.log('✅ Admin user password updated with proper hash'); | ||
| } |
There was a problem hiding this comment.
Plaintext password comparison is ineffective
adminUser.password is hashed; comparing to 'password' will never be true. Either always set a known password during seeding (if desired) or skip resetting existing users.
- // Update the password if it's not hashed (for existing users)
- if (adminUser.password === 'password') {
- const adminUserService = strapi.service('admin::user');
- await adminUserService.updateById(adminUser.id, {
- password: 'password'
- });
- console.log('✅ Admin user password updated with proper hash');
- }
+ // Keep existing admin user as-is to avoid overriding credentials
+ // If you need to reset the password for tests, do it explicitly and unconditionally.
+ // Example (optional):
+ // const adminUserService = strapi.service('admin::user');
+ // await adminUserService.updateById(adminUser.id, { password: 'password' });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Update the password if it's not hashed (for existing users) | |
| if (adminUser.password === 'password') { | |
| const adminUserService = strapi.service('admin::user'); | |
| await adminUserService.updateById(adminUser.id, { | |
| password: 'password' | |
| }); | |
| console.log('✅ Admin user password updated with proper hash'); | |
| } | |
| // Keep existing admin user as-is to avoid overriding credentials | |
| // If you need to reset the password for tests, do it explicitly and unconditionally. | |
| // Example (optional): | |
| // const adminUserService = strapi.service('admin::user'); | |
| // await adminUserService.updateById(adminUser.id, { password: 'password' }); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/cypress.yml (1)
70-73: EnsureCYPRESS_RECORD_KEYsecret is properly configured in the GitHub repository.The Cypress Cloud recording configuration requires
CYPRESS_RECORD_KEYto be set in your GitHub repository secrets. Without this, therecord: trueandparallel: truesettings may cause test failures or skip recording.Verify that:
CYPRESS_RECORD_KEYis configured in the repository's GitHub secrets- The Cypress Cloud project is properly linked in
cypress.config.js(or similar configuration)- Your Cypress Cloud account has parallelization enabled for the project
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/cypress.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cypress-run (2)
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
.github/workflows/cypress.yml (2)
8-14: Parallel execution strategy is well-configured.The matrix strategy with
fail-fast: falseand 2 parallel containers aligns with Cypress parallelization best practices. Tests will now distribute across 2 workers, improving CI feedback speed.
56-59: Script verification confirmed—no changes needed.The
yarn playground:setupscript is correctly defined in root package.json as"yarn --cwd ./playground setup", which invokes the playground'ssetupscript that copies the pre-seeded database file. The workflow is properly configured and the step ordering (directory creation before script execution) is correct.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
cypress/e2e/features.cy.js (2)
77-81: Invalid attribute selector; fix bracket/quote.Selector
'div[role="status'is malformed (missing closing quote and bracket); tests will fail to locate the status element.Apply this diff to fix both selectors:
- cy.get('div[role="status').contains('success').should('be.visible') - cy.get('div[role="status') + cy.get('div[role="status"]').contains('success').should('be.visible') + cy.get('div[role="status"]') .contains('Credentials successfully updated') .should('be.visible')
267-267: Fix selector for title input.Missing closing quote and bracket in attribute selector.
Apply this diff:
- cy.get('input[name="title').type('The slimy snail') + cy.get('input[name="title"]').type('The slimy snail')
🧹 Nitpick comments (3)
.github/workflows/cypress.yml (1)
15-18: Standardize quote style in YAML (optional).The quote style has been switched from single to double quotes for environment variables and port mappings. While both are valid YAML, maintaining consistency across the workflow is good practice. If your team has adopted a quote style convention, ensure it's applied consistently throughout the file.
Also applies to: 26-26
cypress/e2e/features.cy.js (2)
56-61: Replace arbitrary wait with element-based assertion.The hard-coded
cy.wait(1000)is brittle and can cause flakiness. The subsequent assertion already waits for the element to be visible.Apply this diff to remove the unnecessary wait:
validate() { - cy.wait(1000) cy.contains('Hello User who can manage Meilisearch').should( 'be.visible', ) },
171-198: Test interdependencies reduce isolation.These tests explicitly assume previous tests have run (see comments on lines 170, 200). This creates fragile test ordering dependencies.
Consider extracting shared setup logic into helper functions or
before()hooks to ensure each test can run independently. This improves maintainability and allows running individual tests during development.Also applies to: 201-235
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/cypress.yml(2 hunks)cypress/e2e/features.cy.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cypress-run
🔇 Additional comments (3)
.github/workflows/cypress.yml (2)
49-52: LGTM! Seeding strategy improves test resilience.The new "Seed playground database" step prepares a consistent playground state before tests run, supporting the PR objective to make tests more resilient to Strapi UI changes. The step creates the
.tmpdirectory and runs the setup script, which should deterministically initialize the test environment.
61-65: Fix template syntax spacing and verify Cypress recording configuration.The template syntax spacing inconsistency is confirmed—line 63 uses
${{ secrets.CYPRESS_RECORD_KEY }}while line 65 uses${{secrets.GITHUB_TOKEN}}. Additionally, theprojectId: 'vu3oo5'has been confirmed incypress.config.js.To complete the Cypress recording setup:
- Ensure
CYPRESS_RECORD_KEYis configured in your repository secrets- Apply the spacing fix to line 65 for consistency:
env: CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }} # Allows Cypress to detect new build vs re-run build - GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}cypress/e2e/features.cy.js (1)
154-160: Clarify test isolation concern.The test assumes checkboxes are initially unchecked, but if previous tests don't clean up properly or tests run out of order, this will fail. The
cy.fail()call prevents accidental deletion but doesn't help with test isolation.Consider either:
- Adding a test setup/teardown to ensure consistent initial state
- Modifying the test to handle both checked and unchecked states gracefully
- Adding a
.skip()or guard if the checkbox is already in the desired stateVerify whether test ordering and cleanup are guaranteed in your CI environment.
| const checkCollectionContent = ({ rowNb, contains }) => { | ||
| const row = `table[role='grid'] tbody tr:nth-child(${rowNb})` | ||
| contains.map(value => | ||
| cy | ||
| .get(row, { | ||
| timeout: 10000, | ||
| }) | ||
| .contains(value, { timeout: 10000 }), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Fix .map() usage in Cypress command chain.
The .map() method returns a JavaScript array, but it doesn't properly integrate with Cypress command chaining. Each cy.get() call executes independently without proper synchronization.
Apply this diff to use .forEach() instead:
const checkCollectionContent = ({ rowNb, contains }) => {
const row = `table[role='grid'] tbody tr:nth-child(${rowNb})`
- contains.map(value =>
+ contains.forEach(value =>
cy
.get(row, {
timeout: 10000,
})
.contains(value, { timeout: 10000 }),
)
}🤖 Prompt for AI Agents
In cypress/e2e/features.cy.js around lines 35 to 44, the function uses
contains.map(...) which returns a JS array and misuses Cypress command chaining;
replace the .map() with .forEach() so each value is iterated and a
cy.get(...).contains(...) is invoked for side effects, i.e., change
contains.map(...) to contains.forEach(...), keeping the existing cy.get/contains
calls and their timeouts so Cypress executes them in sequence for each value.
| }) | ||
|
|
||
| // This test assumes that the collections are indexed by previous tests | ||
| it('displays the number of inxed documents for each collection', () => { |
There was a problem hiding this comment.
Fix typo in test description.
"inxed" should be "indexed".
Apply this diff:
- it('displays the number of inxed documents for each collection', () => {
+ it('displays the number of indexed documents for each collection', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('displays the number of inxed documents for each collection', () => { | |
| it('displays the number of indexed documents for each collection', () => { |
🤖 Prompt for AI Agents
In cypress/e2e/features.cy.js around line 171 the test description contains a
typo "inxed" instead of "indexed"; update the string in the it(...) call to read
"displays the number of indexed documents for each collection" so the test
message is correct.
what
Refactor Cypress tests to:
how
Summary by CodeRabbit
New Features
Tests
Chores
CI