Skip to content

feat: Integrate Playwright E2E testing infrastructure - #1541

Merged
mariobehling merged 13 commits into
fossasia:devfrom
vedansh-5:playwright
Jul 8, 2026
Merged

feat: Integrate Playwright E2E testing infrastructure#1541
mariobehling merged 13 commits into
fossasia:devfrom
vedansh-5:playwright

Conversation

@vedansh-5

Copy link
Copy Markdown
Member

Description

This Pull Request introduces the base Playwright E2E testing infrastructure to Visdom. As planned, Cypress and its dependencies remain fully intact while both test suites coexist in this transitional phase.

Motivation and Context

Cypress has been prone to flakiness and slower execution speeds. This initial PR establishes the framework and layout for Playwright E2E and visual testing, allowing subsequent PRs to migrate the remaining suites.

How Has This Been Tested?

Tested locally

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [ x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code refactor or cleanup (changes to existing code for improved readability or performance)

Checklist:

  • I adapted the version number under py/visdom/VERSION according to Semantic Versioning
  • [ x ] My code follows the code style of this project.
  • [ x ] My change requires a change to the documentation.
  • [ x ] I have updated the documentation accordingly.

Copilot AI review requested due to automatic review settings July 1, 2026 00:04

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry @vedansh-5, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request introduces an initial Playwright-based E2E testing setup for Visdom (alongside the existing Cypress suite), including a basic smoke-test suite, Playwright configuration, CI wiring, and contributor documentation updates.

Changes:

  • Add Playwright test runner configuration and a starter E2E test spec.
  • Wire a new Playwright job into the existing PR CI workflow and expose an npm script to run it.
  • Update contributor docs/README to document running both Cypress and Playwright tests.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
yarn.lock Adds Playwright packages and updates the dependency lock (also reflects a sizable Cypress dependency reshuffle).
package.json Adds @playwright/test and a test:pw script to run Playwright.
playwright.config.js Introduces Playwright test configuration (baseURL, retries, reporter, project).
playwright/tests/basic.spec.js Adds initial smoke/E2E coverage for basic UI availability and env selection.
playwright/support/helpers.js Adds helper utilities intended for driving demo/env flows in Playwright tests.
.github/workflows/process-changes.yml Adds a new CI job to install Playwright Chromium, start Visdom, and run Playwright tests.
CONTRIBUTING.md Documents how to install and run Playwright E2E tests locally.
README.md Updates the contributing blurb to mention both Cypress and Playwright E2E/visual testing.

Comment thread .github/workflows/process-changes.yml Outdated
Comment thread playwright/tests/basic.spec.js
Comment thread playwright/tests/basic.spec.js

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

New security issues found

Comment thread playwright/support/helpers.js Outdated
Comment thread playwright/support/helpers.js Outdated

@Jayantparashar10 Jayantparashar10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread .github/workflows/process-changes.yml Outdated

@tonypzy tonypzy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please resolve comments

@Jayantparashar10
Jayantparashar10 requested a review from tonypzy July 4, 2026 13:33

@tonypzy tonypzy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it is good right now

@Manik-Khajuria-5 Manik-Khajuria-5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vedansh-5 I went through the code most of things are implemneted properly but in ci server is started in a separate CI step from the tests I think they must be in single step as there are certain times when server hanging situation happens or what if server did not survive at all or there might be another approach please verify it once as I am also not too sure about it !

@vedansh-5

Copy link
Copy Markdown
Member Author

@vedansh-5 I went through the code most of things are implemneted properly but in ci server is started in a separate CI step from the tests I think they must be in single step as there are certain times when server hanging situation happens or what if server did not survive at all or there might be another approach please verify it once as I am also not too sure about it !

Thanks for pointing this out, I have addressed and pushed these changes.

@vedansh-5

Copy link
Copy Markdown
Member Author

@Manik-Khajuria-5 Please take a look at the changes.

@Manik-Khajuria-5 Manik-Khajuria-5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vedansh-5 LGTM! adding in config section is a good choice nice work

@vedansh-5

Copy link
Copy Markdown
Member Author

@Saksham-Sirohi @mariobehling This is ready to be merged, Ptal

@mariobehling
mariobehling merged commit c1d183d into fossasia:dev Jul 8, 2026
6 checks passed
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.

7 participants