Skip to content

test: add ability to do visual regression tests#42

Open
billybonks wants to merge 1 commit intomainfrom
feat/visual-regression-testing
Open

test: add ability to do visual regression tests#42
billybonks wants to merge 1 commit intomainfrom
feat/visual-regression-testing

Conversation

@billybonks
Copy link
Contributor

No description provided.

Copy link

@managerbot-app managerbot-app bot left a comment

Choose a reason for hiding this comment

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

Reviewed Changes

Managerbot reviewed 3 out of 8 changed files in this pull request and generated 4 comments.

Files not reviewed (5)
  • .gitignore
  • package-lock.json
  • package.json
  • _config.yml
  • tests/__snapshots__/visual-regression.spec.ts/home-page.png
Comments suppressed due to low confidence (0)

snapshotPathTemplate: '{testDir}/__snapshots__/{testFilePath}/{arg}{ext}',
expect: {
toHaveScreenshot: {
maxDiffPixelRatio: 0.01,

Choose a reason for hiding this comment

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

Consider using a lower tolerance value for maxDiffPixelRatio. 0.01 (1%) might still allow noticeable visual differences to pass. Depending on your UI complexity, values between 0.001-0.005 might be more appropriate.

try {
await expect(page).toHaveScreenshot('home-page.png', {
fullPage: true,
maxDiffPixelRatio: 0.01,

Choose a reason for hiding this comment

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

This duplicates the maxDiffPixelRatio setting that's already in the global config. Consider removing it here to keep configuration in one place.

await page.goto('/');

// Wait for the page to be fully loaded
await page.waitForLoadState('networkidle');

Choose a reason for hiding this comment

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

networkidle can be flaky for pages with continuous background activity. Consider adding additional stabilization checks or a fixed timeout if needed.

});

// Test passed - remove diff image if it exists
if (fs.existsSync(DIFF_IMAGE)) {

Choose a reason for hiding this comment

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

Consider wrapping file operations in try/catch blocks to handle potential file system errors gracefully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant