LearnNewsExamplesServices
Frontmatter
titleFix #7282 Migrated FunctionalButton test to playwright
authornabeel001
stateMerged
createdAtOct 9, 2025, 6:27 AM
updatedAtOct 9, 2025, 8:21 PM
closedAtOct 9, 2025, 8:19 PM
mergedAtOct 9, 2025, 8:19 PM
branchesdev ← functionalButton-migrate-playwright
urlhttps://github.com/neomjs/neo/pull/7423
Merged
nabeel001
nabeel001 commented on Oct 9, 2025, 6:27 AM

Please make sure to read the Contributing Guidelines:

https://github.com/neomjs/neo/blob/dev/CONTRIBUTING.md

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the dev branch, not the main branch
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information: Fixes #7282 Migrated FunctionalButton test to playwright

tobiu
tobiu commented on Oct 9, 2025, 8:21 PM

thx for your contribution! input from gemini:

✦ I have reviewed the pull request and the original Siesta test file. The diff provided by the web_fetch tool was clean this time, and I was able to perform a thorough comparison.

This is a perfect migration. The contributor has done an excellent job.

Review:

  • File Creation: The new test file is correctly created at test/playwright/unit/functional/Button.spec.mjs.
  • Test Coverage: All four tests from the original Siesta file (should create initial vnode correctly, should update vnode and create delta for a single config change, should update vnode and create delta for multiple config changes, should handle pressed state change) have been migrated.
  • Assertion Translation: All assertions have been correctly and accurately translated from Siesta's t.expect, t.is, t.isDeeply, t.ok, and t.notOk to the equivalent Playwright/Jest expect() syntax (e.g., .toBe(), .toEqual(), .toBeTruthy(), .toBeFalsy()).
  • Code Quality: The new test is clean, well-structured, and uses the project's setup() helper for configuration, which is an improvement over the manual setup in the old test.

Conclusion: This PR fully meets all acceptance criteria from the ticket.

My review comment for the PR will be:

This is a perfect migration, fantastic work!

I've compared the new Playwright test side-by-side with the original Siesta test, and it's a flawless translation.

  • The new file is created at the correct location.
  • The test setup and teardown logic is correctly adapted.
  • All tests and assertions from the original file are present and correctly converted to the expect syntax.
  • The code is clean and follows the project's conventions.

This fully satisfies the requirements of ticket #7282. Great job