Skip to content
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

fix: run Playwright tests in all browsers #6876

Closed
wants to merge 9 commits into from

Conversation

radium-v
Copy link
Collaborator

Pull Request

📖 Description

This PR updates the Playwright config in fast-foundation to run tests in Firefox and Webkit.

This PR also updates Playwright to the latest version, 1.40.1. The custom hasAttribute function has been replaced with Playwright's built-in toHaveAttribute function, which now allows for checking the existence of an attribute without specifying a value.

Some toolbar tests were failing, so I added a focus() method as a workaround.

👩‍💻 Reviewer Notes

Some tests for Design Tokens are failing only in Webkit, so they're skipped for now. Toolbar tests were also failing in Firefox and Webkit, though I couldn't identify if this is a problem with Playwright or with our component. This may be related to these bugs:

📑 Test Plan

All tests should pass as expected.

Toolbar adds a focus() method, but it's marked as @internal.

✅ Checklist

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

Copy link
Collaborator

@bheston bheston left a comment

Choose a reason for hiding this comment

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

Thanks for this update. I'm interested to know what is failing in Safari and to make sure we can figure that out because I feel that one of the benefits of a library like this is that it solves these cross-platform issues. I haven't seen any mention of actual bugs with tokens, but it would be really nice if we knew it was working via tests as well.

Regarding the delegates focus comments, we recently removed that from the registration on Adaptive Web Components. Many of the components are not best served with the current capabilities of this model. For instance, if there is extra whitespace on a custom element, the whole area causes a focus anyway. This was most prominent in the area to the right of the label above an input like on Text Field. We added a focus override to correctly place focus. At least in Foundation this is only part of the test infrastructure and not the export.

@radium-v radium-v force-pushed the users/jokreitl/upgrade-playwright-1.40.0 branch from 0544142 to 9c53725 Compare January 3, 2024 02:33
@bheston
Copy link
Collaborator

bheston commented Jan 3, 2024

It looks like there are some issues in the Calendar tests in Webkit now. It seems to be adding characters to the year portion for some locale tests. Maybe we just check that the result contains the number we're expecting instead.

I suspect the timeouts are spurious, though I still wish there were a way around this.

@radium-v radium-v force-pushed the users/jokreitl/upgrade-playwright-1.40.0 branch from e132a36 to be81ac7 Compare January 17, 2024 18:38
@radium-v radium-v force-pushed the users/jokreitl/upgrade-playwright-1.40.0 branch 2 times, most recently from 9d3d950 to f7d2056 Compare February 4, 2024 08:17
@radium-v radium-v force-pushed the users/jokreitl/upgrade-playwright-1.40.0 branch from f7d2056 to 57f7d08 Compare February 5, 2024 01:42
@radium-v
Copy link
Collaborator Author

radium-v commented Feb 5, 2024

In order to get this PR to pass, I had to set up the build toolchain on Windows to address a Calendar bug. This bug only happens on newer versions of Webkit on Windows. I had to include it with this PR since the bug doesn't appear on the main branch.

There's also a problem when using the most recent version of Playwright in fast-ssr - the style tests which check that the background-color gets set properly on the card components returns rgba(0, 0, 0, 0) instead of rgb(26, 26, 26). I don't know why this bug is happening, so if someone with a better understanding of how fast-ssr works could investigate the issue in the future, that would be awesome.

@radium-v radium-v requested a review from awentzel as a code owner February 5, 2024 03:36
@radium-v radium-v marked this pull request as draft February 12, 2024 02:53
@radium-v
Copy link
Collaborator Author

Converting this back to a draft to split out some of the work.

@radium-v radium-v mentioned this pull request Feb 12, 2024
5 tasks
@radium-v radium-v closed this Feb 13, 2024
@radium-v radium-v deleted the users/jokreitl/upgrade-playwright-1.40.0 branch February 13, 2024 00:56
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.

4 participants