LearnNewsExamplesServices
Frontmatter
title Convert state/ProviderNestedDataConfigs.mjs Test from Siesta to Playwright #7289
authorkart-u
stateMerged
createdAtOct 4, 2025, 12:31 PM
updatedAtOct 4, 2025, 6:33 PM
closedAtOct 4, 2025, 4:28 PM
mergedAtOct 4, 2025, 4:28 PM
branchesdev ← Provider-config-seista-playwright
urlhttps://github.com/neomjs/neo/pull/7351
Merged
kart-u
kart-u commented on Oct 4, 2025, 12:31 PM

Closes #7289

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: ✅ Create a new test file at test/playwright/unit/state/ProviderNestedDataConfigs.spec.mjs ✅ Translated all Siesta assertions (t.is) to Playwright expect syntax ✅ All tests pass successfully with npm test ✅ Generated using follow the instructions inside @AGENTS.md protocol

tobiu
tobiu commented on Oct 4, 2025, 5:03 PM

thx for the contribution, i merged it in.

there was a minor issue: tests did not pass:

Screenshot 2025-10-04 at 16 33 05

this was just a subtle (but mean) difference in how neo works inside the browser vs inside nodejs. since playwright is running sub-tests (even when inside the same file) inside web workers, classes get used without the enhancement from setupClass(), in case we do not re-assign them to the class variable.

Fix commit: https://github.com/neomjs/neo/commit/11c42a5362a72f7f360dc326d2543986d489781f

Let me give you a hint, on how to improve your workflow:

Search the https://github.com/neomjs/neo/tree/dev/.github/ISSUE folder for the github ticket id (#7289 in this case). or tell gemini to find it. Then gemini can read the ticket and related epic: https://github.com/neomjs/neo/blob/dev/.github/ISSUE/epic-enhance-workflow-with-mandatory-unit-testing.md https://github.com/neomjs/neo/blob/dev/.github/ISSUE/ticket-convert-state-providernesteddataconfigs-test.md

=> the epic contains the re-assignment hint, since it is non-trivial (i fell for it too at first).

this also prevents gemini from creating a new ticket md file, since there is already one in place.

kart-u
kart-u commented on Oct 4, 2025, 5:20 PM

thanks for the insights if you do not mind can you correct me if my understanding is wrong ,this issue was arising because each worker loading class's unmodified instance so basically for every test we need to modify class every time at start of each test?

tobiu
tobiu commented on Oct 4, 2025, 6:33 PM

Sounds about right. Let me give you more input: normally, we define each class as its own file. Every file ends with exporting the return value of Neo.setupClass(). To pick an easy example: https://github.com/neomjs/neo/blob/dev/src/component/Image.mjs#L78

Now inside tests as well as code livepreviews, we define classes inline instead for convenience. To behave the same, the re-assignment is key. setupClass() modifies class prototypes at run-time (JS can do this) and also acts as a gatekeeper to combine different neo envs on the same website if needed. Code: https://github.com/neomjs/neo/blob/dev/src/Neo.mjs#L667

I think there is a hacktoberfest sub-ticket, just to write a guide on what this method does, since it is non-trivial, but extremely powerful.