Skip to content

feat(test-plans): add design-fix-test-plan skill and related tests#36124

Open
nicobytes wants to merge 2 commits into
mainfrom
36123-add-e2e-tests-for-file-image-and-binary-fields-in-edit-content-screen
Open

feat(test-plans): add design-fix-test-plan skill and related tests#36124
nicobytes wants to merge 2 commits into
mainfrom
36123-add-e2e-tests-for-file-image-and-binary-fields-in-edit-content-screen

Conversation

@nicobytes

@nicobytes nicobytes commented Jun 11, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Introduces a new skill, design-fix-test-plan, that generates structured test plans for bug fixes and features in dotCMS. The skill operates in three modes: TDD, QA post-fix, and QA pre-fix, allowing users to create test cases with detailed reproduction steps and expected results.

  • Added README.md and SKILL.md documentation for the new skill.
  • Implemented E2E tests for binary, file, and image fields, ensuring functionality for uploading, importing, and validation of required fields.
  • Created helper classes for binary and file fields to streamline test interactions.

Checklist

  • Documentation added
  • E2E tests implemented

Closes #XXXX (replace with relevant issue number if applicable)

Proposed Changes

  • change 1
  • change 2

Checklist

  • Tests
  • Translations
  • Security Implications Contemplated (add notes if applicable)

Additional Info

** any additional useful context or info **

Screenshots

Original Updated
** original screenshot ** ** updated screenshot **

This PR fixes: #36123

### Proposed Changes

Introduces a new skill, **design-fix-test-plan**, that generates structured test plans for bug fixes and features in dotCMS. The skill operates in three modes: TDD, QA post-fix, and QA pre-fix, allowing users to create test cases with detailed reproduction steps and expected results.

- Added README.md and SKILL.md documentation for the new skill.
- Implemented E2E tests for binary, file, and image fields, ensuring functionality for uploading, importing, and validation of required fields.
- Created helper classes for binary and file fields to streamline test interactions.

### Checklist
- [x] Documentation added
- [x] E2E tests implemented

Closes #XXXX (replace with relevant issue number if applicable)
@claude

claude Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @nicobytes's task in 1m 39s —— View job


Code Review

  • Read changed files
  • Analyze and provide review

E2E Tests

1. Assertion is too weak — allows 4xx/5xx to pass

binary-field/helpers/binary-field.ts:119 and file-field/helpers/file-field.ts:159:

expect(response.status()).not.toBe(400);

A 500, 503, or 404 passes this check. Use expect(response.ok()).toBeTruthy() (checks 200–299) instead. Fix this →

2. expectPreviewShowsFileName silently skips the filename assertion

binary-field/helpers/binary-field.ts:69–79:

async expectPreviewShowsFileName(fileName: string) {
    const codePreview = this.preview.getByTestId('code-preview');
    if (await codePreview.isVisible()) {
        await expect(codePreview).toBeVisible({ timeout: 15000 });
        return;  // ← exits without checking fileName
    }
    // ...
}

When code-preview is visible the fileName parameter is ignored entirely. The happy path for text files never validates the filename. Same issue exists in file-field/helpers/file-field.ts:87–96. Fix this →

3. Cross-field helper dependency

binary-field/helpers/binary-field.ts:1–9:

import { E2E_IMPORT_URL, REQUIRED_FIELD_ERROR, createTestTextFile }
    from '../../file-field/helpers/file-field';
export { E2E_IMPORT_URL, createTestTextFile };

BinaryField directly imports from FileField's helper directory. Sibling field helpers should not depend on each other — shared constants (E2E_IMPORT_URL, REQUIRED_FIELD_ERROR, createTestTextFile, createTestPngFile) belong in helpers/ or utils/ at the edit-content/fields/ level. Fix this →

4. CSS selector used outside Dojo iframe context

file-field/helpers/file-field.ts:130–135:

const closeButton = dialog
    .locator('.p-dialog-header-close, .p-dialog-close-button, button[aria-label="Close"]')
    .first();

Per AGENTS.md locator priority (getByRole → getByTestId → getByLabel → CSS only in Dojo iframe), this violates project conventions. Should be dialog.getByRole('button', { name: 'Close' }). Fix this →

5. BinaryField.requiredError redundantly re-scopes the root

binary-field/helpers/binary-field.ts:40–42:

this.requiredError = page
    .locator(`dot-binary-field-wrapper[data-testid="field-${fieldVariable}"]`)
    .locator('.error-message small');

this.root is already page.getByTestId('field-${fieldVariable}') and scopes to the same element. Should be this.root.locator('.error-message small') to match how FileField handles it. Fix this →

6. uniqueSuffix() copy-pasted across three spec files

Lines 18–20 of binary-field.spec.ts, file-field.spec.ts, and image-field.spec.ts are byte-identical. Extract to utils/ or a shared test fixture.

7. BinaryField.importFromUrl has no timeout on waitForResponse

binary-field/helpers/binary-field.ts:108–113: no timeout is set. FileField.importFromUrl sets { timeout: 30000 } on the workflow response — the temp-URL response in BinaryField could hang indefinitely on a slow CI environment. Add a consistent timeout. Fix this →

8. Reload test bypasses the helper abstraction

binary-field.spec.ts:81–83:

await expect(field.preview.getByTestId('code-preview')).toContainText(
    'dotCMS E2E test file content'
);

BinaryField.expectPreviewShowsFileName exists for exactly this purpose but isn't called here. Consistent use of the helper's public API makes tests more resilient to selector changes. (Same concern in file-field.spec.ts:81–83 which also accesses code-preview directly.)


Skill Files (.agents/skills/design-fix-test-plan/)

These are clean and comprehensive. No structural issues.

… matching

### Changes Made
- Consolidated import statements in `binary-field.spec.ts` and `image-field.spec.ts` for improved readability.
- Reformatted the URL matching logic to enhance clarity and maintainability in both test files.

### Checklist
- [x] Code readability improved
- [x] No functional changes introduced

This refactor aims to enhance the maintainability of the E2E tests for binary and image fields without altering their functionality.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Add E2E tests for File, Image, and Binary fields in Edit Content Screen

1 participant