Skip to content

Playwright component testing#588

Merged
bedrich-schindler merged 6 commits into
nextfrom
task/587
Apr 15, 2025
Merged

Playwright component testing#588
bedrich-schindler merged 6 commits into
nextfrom
task/587

Conversation

@bedrich-schindler

Copy link
Copy Markdown
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

bedrich-schindler commented Feb 15, 2025

Copy link
Copy Markdown
Contributor Author

@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.

Comment thread .env.playwright.dist
Comment thread .github/workflows/test.yml Outdated
Comment thread src/components/Button/__tests__/Button.spec.jsx
Comment thread src/components/Button/__tests__/Button.spec.jsx
Comment thread src/components/Button/__tests__/Button.story.jsx
Comment thread tests/playwright/propTests/labelVisibilityPropTest.js

@mbohal mbohal 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.

I really, really like this!

Comment thread CONTRIBUTING.md Outdated
Comment thread package.json
Comment thread tests/playwright/components/TestIcon/TestIcon.jsx
Comment thread tests/playwright/constants/breakpoints.js Outdated
Comment thread package.json Outdated
Base automatically changed from maintenance/github-workflows-upgrade-ubuntu to master February 19, 2025 08:28
@bedrich-schindler

Copy link
Copy Markdown
Contributor Author

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

@bedrich-schindler

Copy link
Copy Markdown
Contributor Author

@mbohal and @adamkudrna, ready for round 2.

@bedrich-schindler

Copy link
Copy Markdown
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.

@adamkudrna adamkudrna 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.

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! 👏🏻

Comment thread .github/workflows/test.yml
Comment thread .gitignore Outdated
Comment thread .github/workflows/release-management.yml Outdated
Comment thread .github/workflows/release-management.yml Outdated
Comment thread TESTING.md Outdated
Comment thread TESTING.md Outdated
Comment thread TESTING.md Outdated
Comment thread package.json
Comment thread .github/workflows/test.yml Outdated
Comment thread .github/workflows/playwright.yml
Comment thread TESTING.md Outdated
Comment thread TESTING.md Outdated
@bedrich-schindler

bedrich-schindler commented Feb 25, 2025

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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.

Comment thread tests/playwright/templates/index.html
Comment thread TESTING.md Outdated
by using the following command:

```bash
npm run test:playwright-ct:show-report

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.

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
Copy Markdown
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!

Comment thread CONTRIBUTING.md Outdated
Comment thread CONTRIBUTING.md Outdated
Comment thread TESTING.md Outdated

Test report cannot be server from Docker container yet.

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

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.

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
Copy Markdown
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.

Comment thread TESTING.md Outdated
@bedrich-schindler

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.
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