Overview
This skill produces structured code review reports for Cypress E2E tests written against the FOLIO Stripes framework. Reviews are output as a Markdown file named <BRANCH_NAME_CODEREVIEW>.md.
Quick Reference
| Task | Command |
|---|---|
| Get changed files | git diff --name-only master |
| Get full diff | git --no-pager diff --no-prefix --unified=100000 --minimal origin/master...HEAD |
| Output file | <BRANCH_NAME_CODEREVIEW>.md |
Step-by-Step Workflow
Step 1 — Gather the diff
Run both commands before reviewing anything. Never review partial diffs.
# 1. List changed files
git diff --name-only master
# 2. Get the complete diff (all changed files, full context)
git --no-pager diff --no-prefix --unified=100000 --minimal origin/master...HEAD
Diff line prefix legend:
| Prefix | Meaning |
|---|---|
+ |
Line added |
- |
Line removed |
(space) |
Line unchanged |
@@ |
Hunk header |
Step 2 — Determine the branch name
Use the branch name as the output filename: <BRANCH_NAME_CODEREVIEW>.md.
Step 3 — Review against all checklist items (below)
Step 4 — Write the report using the required format (below)
Review Checklist
Work through every item for every changed file before writing the report.
1. Fragment & Helper Reuse
- Are existing fragments, functions, and utilities being used rather than duplicated?
- Search
cypress/support/fragments/for relevant page objects before flagging missing abstractions. - Is logic that could live in a fragment written inline in the spec instead?
2. Interactors vs Raw Selectors
Prefer interactors from @interactors/html over raw Cypress selectors.
// ✅ Preferred
cy.do(Button('Save').click());
cy.expect(TextField('Name').has({ value: 'test' }));
// ❌ Avoid when an interactor exists
cy.get('[data-testid="save-button"]').click();
Flag every raw selector that has a known interactor equivalent.
3. Describe Block Hierarchy
Must mirror the FOLIO module/feature hierarchy:
describe('Module', () => {
describe('Feature Area', () => {
describe('Specific Function', () => {
it('C##### Test name', { tags: [...] }, () => { ... });
});
});
});
Flag: missing levels, incorrect order, or hierarchy that doesn't match the test case path.
4. Tags
Every it block must have exactly 3 tag components:
{ tags: ['extendedPath', 'spitfire', 'C627455'] }
| Position | Valid values |
|---|---|
| Test group | smoke, criticalPath, extendedPath; append ECS for consortia |
| Dev team | lowercase team name: spitfire, thunderjet, eureka, etc. |
| Test ID | C###### exact identifier |
Flag: missing tags, wrong order, wrong group name, missing test ID.
5. Test Data Cleanup
- All entities created in the test must be deleted in
after()via API. - Default / pre-existing entities must NOT be deleted: service points, locations, material types, loan types, institutions, campuses, libraries, etc.
- Check that
after()exists and covers every created entity.
6. Import Paths
- Verify all import paths are correct and consistent with the project structure.
- Flag relative paths that should be absolute (or vice versa per project convention).
- Flag imports of non-existent or renamed modules.
7. Forbidden Patterns
| Pattern | Rule |
|---|---|
cy.pause() |
Never allowed in committed code |
cy.wait(ms) |
Flag every usage; prefer fragment waitLoading() methods |
Hardcoded test data without getRandomPostfix() |
Flag — causes data conflicts in parallel runs |
8. Code Duplication & Reusability
- Is logic copy-pasted from another test that could be extracted to a fragment?
- Are
before()/after()hooks unnecessarily duplicated acrossitblocks?
9. Naming Conventions
- Test data:
AT_C######_Description_${randomPostfix} - Fragment methods: verb-noun camelCase (
verifyRecordExists,fillName,openForm) - Spec files: lowercase hyphenated (
call-number-browse.cy.js) - Variables: descriptive camelCase, no single-letter names outside loops
10. Parallel Run Safety
- Test data must be unique per run (use
getRandomPostfix()). - No shared mutable state between
itblocks. - No reliance on test execution order.
Report Format
Output a Markdown file with this exact structure:
# Code Review for ${feature_description}
Brief overview: purpose of the change, relevant context, files involved.
# Suggestions
## ${emoji} ${Summary of suggestion}
* **Priority**: ${🔥 / ⚠️ / 🟡 / 🟢}
* **File**: `relative/path/to/file.cy.js`
* **Details**: Clear explanation of the issue and why it matters.
* **Example** (if applicable): what the current code does wrong.
* **Suggested Change** (if applicable):
```javascript
// improved code here
```
## (next suggestion...)
# Summary
High-level recap: overall quality, top issues to fix, positive highlights.
Priority Levels
| Emoji | Level | When to use |
|---|---|---|
| 🔥 | Critical | Breaks tests, causes false positives/negatives, data leaks between runs |
| ⚠️ | High | Violates required standards (missing tags, missing cleanup, cy.pause()) |
| 🟡 | Medium | Code quality issues, raw selectors with interactor equivalents, duplicated logic |
| 🟢 | Low | Naming, style, minor readability improvements |
Suggestion Type Emojis
| Emoji | Code | When to use |
|---|---|---|
| 🔧 | :wrench: |
Change required — concern or refactor worth addressing |
| ❓ | :question: |
Genuine question requiring a response with sufficient context |
| ⛏️ | :pick: |
Nitpick — no action required; stylistic or formatting |
| ♻️ | :recycle: |
Refactor suggestion — actionable, not a nitpick |
| 💭 | :thought_balloon: |
Concern, alternative solution, or walkthrough for understanding |
| 👍 | :+1: |
Genuine praise for something well thought out — use sparingly |
| 📝 | :memo: |
Explanatory note or relevant context; no action needed |
| 🌱 | :seedling: |
Observation with future implications; not a change request |
Example Suggestion Blocks
Raw selector instead of interactor
## ♻️ Replace raw selector with interactor
* **Priority**: 🟡
* **File**: `cypress/e2e/inventory/settings/call-number-browse.cy.js`
* **Details**: A raw Cypress selector is used where a built-in interactor exists. Raw selectors are brittle and couple tests to DOM implementation details.
* **Example**: `cy.get('[data-testid="save-button"]').click();`
* **Suggested Change**:
```javascript
cy.do(Button('Save').click());
### Missing cleanup in after()
```markdown
## 🔧 Created entity not deleted in after() hook
* **Priority**: ⚠️
* **File**: `cypress/e2e/orders/order-lines.cy.js`
* **Details**: `testData.organization` is created via API in `before()` but never deleted in `after()`. This pollutes the environment and can cause failures in subsequent runs.
* **Suggested Change**:
```javascript
after(() => {
cy.getAdminToken().then(() => {
Organizations.deleteOrganizationViaApi(testData.organization.id);
});
});
### cy.pause() present
```markdown
## 🔧 Remove cy.pause() before merging
* **Priority**: 🔥
* **File**: `cypress/e2e/inventory/items/item-create.cy.js`
* **Details**: `cy.pause()` halts test execution and must never be committed. This will cause CI runs to hang indefinitely.
Critical Rules
- Always run both git commands before starting the review — never review from memory or partial context.
- Never flag suppressed warnings (
#pragma warning disableequivalents) — assume they are intentional. - Address TODO comments found in the diff — include them as suggestions.
- Do not overwhelm — prioritize 🔥 and ⚠️ items; group related 🟡/🟢 items where possible.
- Always include file paths in every suggestion.
- Output file name:
<BRANCH_NAME_CODEREVIEW>.md— derive branch name from git context.