Migrate draggable block tests to Playwright #43798
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +25 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
|
I don't know for sure, but my hunch is that playwright's implementation of drag and drop isn't handling the I believe @kevin940726 has implemented file drag and drop in #42722, so there may be some inspiration there, but it is a lot of code. 😬 |
|
Thanks, @talldan. I just saw your issue in the Playwright repo microsoft/playwright#16437. Maybe we can extract helper from #42722 before the official API. |
Mamaduka
commented
Sep 5, 2022
Mamaduka
commented
Sep 5, 2022
Mamaduka
commented
Sep 5, 2022
talldan
reviewed
Sep 6, 2022
Mamaduka
commented
Sep 6, 2022
talldan
reviewed
Sep 6, 2022
| secondParagraphBound.y + secondParagraphBound.height * 0.75 | ||
| ); | ||
| await secondParagraph.hover( { |
There was a problem hiding this comment.
@kevin940726 It seems inconsistent that the first test uses only hover and the second uses only mouse.move. What's the reason behind that?
It'd be good to find a fairly standardized approach so that adding more tests later on is easier.
There was a problem hiding this comment.
Partly because the second test requires us to calculate the position based on the block's height. Another reason is that hover by default will check for the element's "actionability", but for whatever reason the second test will and only will fail in headless mode, I'm not sure why yet. We can use force to bypass the check though, but it might be more difficult to read for people not that familiar with Playwright's advanced API.
I agree there's inconsistency though, maybe we should both use mouse.move?
There was a problem hiding this comment.
In my testing the second test failed unless using both hover and move. It might be that the first test was only passing because the mouse is already in the right position to drop above the first block when dragging starts.
|
I get similar failures when running tests locally. |
|
It looks like the @kevin940726, adding an inline comment about the |
talldan
force-pushed
the
migrate/draggable-blocks-tests
branch
from
8a4c5e5
to
2d99682
Compare
|
Pushed a commit that removes the skipped puppeteer test. |
talldan
requested review from
ntwb,
nerrad,
ajitbohra and
ellatrix
as code owners
talldan
added
the
[Type] Automated Testing
|
The failing PHP tests were fixed by #43927. I could rebase and re-run the tests here, but I'll merge with the failing PHP test because I'm impatient. |
This was referenced Sep 7, 2022
talldan
changed the title
|
Thank you, @kevin940726 and @talldan, for driving the PR forward! |
|
And we have our first flaky test report - #43932. I'll keep an eye on this one. |
ntsekouras
reviewed
Sep 13, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
I started working on this migration in hopes of fixing #43737. Unfortunately can't get dragging work correctly.
I also tried using
page.dragAndDropand logging the following message for failure -subtree intercepts pointer events.Sharing current code, maybe a fresh perspective can help me unblock.
Cc @kevin940726, @ciampo, @talldan, @WunderBart
Why?
How?
Testing Instructions
Running the test:
Check the trace:
Screenshots or screencast