Skip to content

Playwright component testing #588

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.

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
merged 6 commits into from
Apr 15, 2025
Merged

Playwright component testing #588

merged 6 commits into from
Apr 15, 2025

Conversation

bedrich-schindler
Copy link
Contributor

I introduced Playwright Component Testing as replacement for Jest and RTL.

Those tests must be run within `playwright` docker container with pre-installed
browsers to ensure uniform testing environment. `.env.playwright` is created
using `postinstall` script to follow zero-config setup. `.env.playwright` can
be used to tween parameters for local development if needed.

Currently, only tests for `Alert` and `Button` components are migrated to
Playwright CT as this commit is supposed to be proof-of-concept. Rest should
be migrated in later pull request.

All visual tests use prop tests concept we already have for Jest to simplify
testing and to make it DRY. `mixPropTests()` is introduced to mix those
to create combinations to be tested.

@bedrich-schindler bedrich-schindler added refactoring testing Anything related just to testing labels Feb 15, 2025
@bedrich-schindler bedrich-schindler self-assigned this Feb 15, 2025
@bedrich-schindler bedrich-schindler changed the title Task/587 Playwright component testing Feb 15, 2025
@bedrich-schindler bedrich-schindler force-pushed the task/587 branch 3 times, most recently from 591decf to 92827da Compare February 15, 2025 18:55
@bedrich-schindler bedrich-schindler changed the base branch from master to maintenance/github-workflows-upgrade-ubuntu February 15, 2025 18:56
@bedrich-schindler
Copy link
Contributor Author

bedrich-schindler commented Feb 15, 2025

@literat, this PR might be interesting for you as it shows Playwright Component Testing in action I talked about on Wednesday. As we sometimes share some concepts between React UI and Spirit, do not hesitate to add some comments to this PR.

Copy link
Contributor

@mbohal mbohal left a comment

Choose a reason for hiding this comment

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

I really, really like this!

Base automatically changed from maintenance/github-workflows-upgrade-ubuntu to master February 19, 2025 08:28
@bedrich-schindler
Copy link
Contributor Author

I rebased it as there were conficts in package-lock.json

@bedrich-schindler
Copy link
Contributor Author

@mbohal and @adamkudrna, ready for round 2.

@bedrich-schindler
Copy link
Contributor Author

One more thing. I created follow up for better organization of test runners that way beyond this pull request:

#590

Part of the issue is uniform environment to prevent unexpected errors on Apple Silicon where you need to run npm ci inside Docker.

Copy link
Member

@adamkudrna adamkudrna left a comment

Choose a reason for hiding this comment

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

The tests alone look good, however I have some major comments on the dev stack and the docs.

No matter that, having finally the visual tests is so cool! 👏🏻

@bedrich-schindler
Copy link
Contributor Author

bedrich-schindler commented Feb 25, 2025

As I mentioned in comments, there is follow-up issue that tracks improvements that need to be discussed and processed on Monday's call: #590 (comment)

If there is not blocking problem, we should close this as soon as it gets and work on improvements later not to block further work as none of the comment is related to writing tests but to stack around it.

Anyway, I thank @adamkudrna for the discussions he raised. I can sound uncompromising, but I must balance between the things that are necessary and that are not to not to block further work and to keep the scope as tight as possible as there are thousands of things that might be solved in this PR.

@bedrich-schindler
Copy link
Contributor Author

Pull request should be ready for next round @adamkudrna.

I also told @lukasbriza to check the discussions again to bring next opinion into discussion.

TESTING.md Outdated
by using the following command:

```bash
npm run test:playwright-ct:show-report
Copy link
Contributor

Choose a reason for hiding this comment

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

I have written this to a comment under outdated yet unresolved discussion. I will put it here as well, just to be sure it does not disappear.

To serve the report I just run cd playwright-report and then python -m http.server 8000.
It then serves the report on port 8000.

I find this better:

  • port is set inline, no need for the PW_REPORT_PORT const and non-pretty code in package.json
  • better separation of concerns as playwright creates a report and then another tool serves it
  • On *NIX systems Python is typically installed by default, Node.js is not.

I think we should add this method to docs. I would even consider removing the current one, but I have no strong feelings about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I managed to make it working. I was able to remember that there is actually problem when running docker compose run. The problem is that docker compose run does not expose ports by design and to expose them, --service-ports must be used. That was reason that it was not working before. It is working now and port can be changed using COMPOSE_PLAYWRIGHT_REPORT_PORT. I use default ports 9323 as it is not easy to use read variables from multiple env files (.env + .env.playwright). So it now can be served from Docker!

TESTING.md Outdated

Test report cannot be server from Docker container yet.

[gh-gg-node-shell]: /docs/contribute/general-guidelines#node-shell
Copy link
Contributor

Choose a reason for hiding this comment

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

These links do not work outside of the context of mkdocs. We need to add src to the path.

I dislike having this text twice in the repo, but this also goes for CONTRIBUTING.md so I think it it beyond the scope of this issue -> I will raise it elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I told @mbohal that this is due to fact that this file is symlinked in the same manner as contributing.md, releasing.md and thus this is necessary. We should review this in separate issue. @mbohal should have put this onto his list.

@bedrich-schindler
Copy link
Contributor Author

@mbohal, there was error in CONTRIBUTING.md. Instead of:

docker compose run --rm node_shell -c 'npm <command>'

there was:

docker compose run --rm node_shell bash -c 'npm <command>'

That resulted in error as there is entrypoint: bash in docker-compose.yml. I fixed that and changed docker-compose.yml to have same configuration for playwright, so there is same approach for both.

@react-ui-org react-ui-org deleted a comment from adamkudrna Mar 4, 2025
@bedrich-schindler bedrich-schindler dismissed adamkudrna’s stale review March 4, 2025 22:01

For some reason, there is 1 requested change, but Github does not show which one. Probably error in the same manner I already reported to Microsoft last week. The only option is to dismiss review to get rid of error.

@bedrich-schindler
Copy link
Contributor Author

⚠️⚠️⚠️ Do not merge this PR until @adamkudrna release new version ⚠️⚠️⚠️

I merged TypeScript support for Playwright into this branch/pull request, squashed commits and rebased it. @lukasbriza, make all changes from this branch (not from master).

@bedrich-schindler
Copy link
Contributor Author

@adamkudrna, when you have time, you can review this. Without your permission, we would not be able to merge it as you are codeowner.

bedrich-schindler and others added 6 commits April 15, 2025 10:11
I introduced Playwright Component Testing as replacement for Jest and RTL.

Those tests must be run within `playwright` docker container with pre-installed
browsers to ensure uniform testing environment. `.env.playwright` is created
using `postinstall` script to follow zero-config setup. `.env.playwright` can
be used to tweak parameters for local development if needed.

`.env` newly contains Playwright related configuration for Docker.

Currently, only tests for `Alert` and `Button` components are migrated to
Playwright CT as this commit is supposed to be proof-of-concept. Rest should
be migrated in later pull request.

All visual tests use prop tests concept we already have for Jest to simplify
testing and to make it DRY. `mixPropTests()` is introduced to mix those
to create combinations to be tested.

New Github workflow is introduced to test only Playwright as it can be
time exhausting. Further improvements are expected to be make in the future.
Transform Playwright related files to Typescript and introduce temporary
configuration files `.eslintrc-ts`, `jest.config-ts.js` and `tsconfig.json`
that must be removed when whole package is transformed into Playwright.

Due to missing TypeScript support in the rest of the package, `*.spec.tsx`
and `*.story.tsx` file do not have complete types and might not be compatible
with strict type checking that should be introduced in #394.

The reason for such change is parallel work on transformation to TypeScript
and on Playwright tests.
@bedrich-schindler bedrich-schindler merged commit 9c7707f into next Apr 15, 2025
10 checks passed
@bedrich-schindler bedrich-schindler deleted the task/587 branch April 15, 2025 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature refactoring testing Anything related just to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants