Skip to content

test: deflake templates locale coverage #7705

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.

Sign up for GitHub

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

Merged
christian-byrne merged 4 commits into from
Dec 28, 2025
Merged

test: deflake templates locale coverage #7705

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1c46de5
test: deflake templates locale coverage
christian-byrne Dec 22, 2025
2533f0d
test: tighten templates locale assertions
christian-byrne Dec 22, 2025
8cf725b
test: anchor locale assertions to dialog
christian-byrne Dec 22, 2025
72799fc
test: scope dialog locator by heading
christian-byrne Dec 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions browser_tests/tests/templates.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,22 +109,27 @@ test.describe('Templates', () => {
})

test('Uses proper locale files for templates', async ({ comfyPage }) => {
// Load the templates dialog and wait for the French index file request
const requestPromise = comfyPage.page.waitForRequest(
'**/templates/index.fr.json'
)

// Set locale to French before opening templates
await comfyPage.setSetting('Comfy.Locale', 'fr')

await comfyPage.executeCommand('Comfy.BrowseTemplates')

const request = await requestPromise
const dialog = comfyPage.page.getByRole('dialog').filter({
has: comfyPage.page.getByRole('heading', { name: 'Modèles', exact: true })
})
await expect(dialog).toBeVisible()

// Verify French index was requested
expect(request.url()).toContain('templates/index.fr.json')
// Validate that French-localized strings from the templates index are rendered
await expect(
dialog.getByRole('heading', { name: 'Modèles', exact: true })
).toBeVisible()
await expect(
dialog.getByRole('button', { name: 'Tous les modèles', exact: true })
).toBeVisible()

await expect(comfyPage.templates.content).toBeVisible()
// Ensure the English fallback copy is not shown anywhere
await expect(
comfyPage.page.getByText('All Templates', { exact: true })
).toHaveCount(0)
})
Comment on lines 111 to 133
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Excellent improvement to test reliability!

The approach of validating rendered French UI strings is much more robust than waiting on network requests. The dialog filtering by heading "Modèles" and use of specific role-based selectors with exact: true aligns well with Playwright best practices.

Optional: Scope the negative assertion to the dialog for consistency.

Lines 122-127 correctly scope positive assertions to the dialog locator, but line 131 searches the entire page. For better test isolation and consistency, consider scoping the English fallback check to the dialog as well:

🔎 Proposed refinement 
     // Ensure the English fallback copy is not shown anywhere
     await expect(
-      comfyPage.page.getByText('All Templates', { exact: true })
+      dialog.getByText('All Templates', { exact: true })
     ).toHaveCount(0)

This ensures the test only validates the dialog content (as stated in the PR objectives) and prevents false negatives if "All Templates" appears elsewhere on the page (e.g., in navigation or menus).

🤖 Prompt for AI Agents 
In browser_tests/tests/templates.spec.ts around lines 111 to 133, the negative
assertion that English fallback text "All Templates" is checked against the
entire page (line ~131), which can produce false negatives; scope that negative
assertion to the dialog locator used above by replacing the page-level getByText
with dialog.getByText (or an equivalent dialog-scoped locator) so the test
verifies that "All Templates" is not present inside the dialog only.


test('Falls back to English templates when locale file not found', async ({
Expand Down
Loading