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.

Sign up for GitHub

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

Jump to bottom

[TEST] Fix panning tests on webkit #1237

Merged
merged 13 commits into from
May 4, 2021
Merged

[TEST] Fix panning tests on webkit #1237

merged 13 commits into from
May 4, 2021

Conversation

tbouffard
Copy link
Member

@tbouffard tbouffard commented Apr 16, 2021 •
edited by csouchet
Loading

No description provided.

@tbouffard tbouffard added chore Build, CI/CD or repository tasks (issues/PR maintenance, environments, ...) WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft labels Apr 16, 2021
@tbouffard
Copy link
Member Author

On Ubuntu (not the targeted OS)

  • Reproduced locally when running tests
  • Manual usage of the playwright webkit browser doesn't spot the issue --> seems to be related to how the tests are written

@tbouffard tbouffard mentioned this pull request Apr 16, 2021
Merged
@csouchet
Copy link
Member

There is a known issue on Playwright for the panning (dragging & dropping): microsoft/playwright#1094

@csouchet csouchet force-pushed the infra/webkit_panning_tests branch 4 times, most recently from 22fdb97 to f962b47 Compare April 30, 2021 09:54
@csouchet csouchet marked this pull request as ready for review April 30, 2021 12:56
@csouchet csouchet removed the WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft label Apr 30, 2021
@csouchet csouchet requested a review from aibcmars April 30, 2021 12:56
csouchet
csouchet reviewed May 3, 2021
View reviewed changes
test/e2e/overlays.rendering.test.ts Outdated
@@ -36,21 +36,23 @@ class ImageSnapshotThresholds extends MultiBrowserImageSnapshotThresholds {
'overlays.start.flow.task.gateway',
{
linux: 0.001, // 0.09368089665046096%
windows: 0.0003, // 0.025623788967854555%
windows: 0.003, // 0.24558800934610941%
Copy link
Member

Choose a reason for hiding this comment

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

I didn't change anything for Chromium on windows. So it's weird to have to change it 🤔

Copy link
Member

@csouchet csouchet May 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@aibcmars Can you verify if you have the same issue on the master branch ?
I updated the version of jest-image-snapshot this morning.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes checking on master - I had like over 20 failures on local :(

Copy link
Contributor

Choose a reason for hiding this comment

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

I confirm that it is not passing on master as well
image

Copy link
Member

Choose a reason for hiding this comment

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

So, I think we should fix that problem in another PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree that this should be done in a dedicated PR

Copy link
Member

Choose a reason for hiding this comment

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

I reverted all the Tresholds commits.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have tested on windows 10 pro locally and don't get any e2e test errors with the original thresholds. This seems an environment issue.

@csouchet
Revert "thresholds"
ef6ddd4 
This reverts commit 2fe969f.
This reverts commit 136fbdc.
This reverts commit 26a2605.
@csouchet
revert HEADLESS type
73ccd3c
@sonarcloud
Copy link

sonarcloud bot commented May 4, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

tbouffard
tbouffard commented May 4, 2021
View reviewed changes
Copy link
Member Author

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

LGTM, I like the refactoring to aling zoom and panning test support 👍🏿

@tbouffard tbouffard merged commit 23264bd into master May 4, 2021
@tbouffard tbouffard deleted the infra/webkit_panning_tests branch May 4, 2021 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csouchet csouchet csouchet approved these changes

@aibcmars aibcmars Awaiting requested review from aibcmars

Assignees
No one assigned
Labels
chore Build, CI/CD or repository tasks (issues/PR maintenance, environments, ...)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tbouffard @csouchet @aibcmars