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

[Flaky Test] should copy only partial selection of text blocks #40303

Closed
github-actions bot opened this issue Apr 13, 2022 · 8 comments · Fixed by #40607
Closed

[Flaky Test] should copy only partial selection of text blocks #40303

github-actions bot opened this issue Apr 13, 2022 · 8 comments · Fixed by #40607
Assignees
Labels
[Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Flaky Test Auto-generated flaky test report issue

Comments

@github-actions
Copy link

github-actions bot commented Apr 13, 2022

Flaky test detected. This is an auto-generated issue by GitHub Actions. Please do NOT edit this manually.

Test title

should copy only partial selection of text blocks

Test path

/test/e2e/specs/editor/various/copy-cut-paste.spec.js

Errors

[2022-04-13T10:32:59.201Z] Test passed after 1 failed attempt on trunk.
[2022-04-18T09:51:38.785Z] Test passed after 1 failed attempt on update/post-featured-image-component.
[2022-04-18T10:53:30.411Z] Test passed after 1 failed attempt on add/wp-env/wp-env-core-env.
[2022-04-18T12:20:51.800Z] Test passed after 1 failed attempt on fix/e2e-test.
[2022-04-18T13:47:15.612Z] Test passed after 1 failed attempt on trunk.
[2022-04-18T13:48:06.669Z] Test passed after 1 failed attempt on fix/duotone-site-editor.
[2022-04-18T14:01:10.151Z] Test passed after 1 failed attempt on fix/playwright-snapshots.
[2022-04-18T14:12:20.129Z] Test passed after 1 failed attempt on trunk.
[2022-04-18T14:48:36.649Z] Test passed after 1 failed attempt on trunk.
[2022-04-18T14:56:37.960Z] Test passed after 1 failed attempt on rnmobile/feature/drag-and-drop.
[2022-04-18T15:02:27.183Z] Test passed after 1 failed attempt on rnmobile/feature/drag-and-drop-prevent-text-focus-on-long-press.
[2022-04-18T15:34:59.990Z] Test passed after 1 failed attempt on add/section-concept.
[2022-04-18T15:38:30.538Z] Test passed after 1 failed attempt on trunk.
[2022-04-18T15:48:40.494Z] Test passed after 1 failed attempt on rnmobile/feature/drag-and-drop-refactor-draggable.
[2022-04-18T15:50:50.420Z] Test passed after 1 failed attempt on rnmobile/feature/drag-and-drop-update-chip-animation.
[2022-04-18T15:52:48.948Z] Test passed after 1 failed attempt on fix/comment-reply-link-alignment.
[2022-04-18T15:55:31.386Z] Test passed after 1 failed attempt on rnmobile/feature/drag-and-drop-haptic-feedback.
[2022-04-18T16:09:33.323Z] Test passed after 1 failed attempt on trunk.
[2022-04-18T16:17:20.282Z] Test passed after 1 failed attempt on fix/embed-block-preview-cut-off.
[2022-04-18T17:30:31.287Z] Test passed after 1 failed attempt on trunk.
[2022-04-18T17:51:21.537Z] Test passed after 1 failed attempt on fix/copy-from-non-text-inputs.
[2022-04-18T19:54:17.115Z] Test passed after 1 failed attempt on remove/block-styles-remove-role.
[2022-04-18T21:02:44.582Z] Test passed after 1 failed attempt on docgen/replace-fixtures-with-code.
[2022-04-18T22:38:25.644Z] Test passed after 1 failed attempt on try/use-css-var-for-user-presets.
[2022-04-23T16:41:35.814Z] Test passed after 1 failed attempt on fix/flaky-test-reporter.
[2022-05-10T13:20:09.628Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-10T13:33:14.282Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-10T13:40:06.465Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-10T15:45:55.917Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-10T15:54:10.994Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-10T18:58:47.148Z] Test passed after 1 failed attempt on fix/input-field-reset-behavior-moar.
[2022-05-16T07:23:49.752Z] Test passed after 1 failed attempt on refactor/range-control-to-typescript.
[2022-05-16T07:50:53.486Z] Test passed after 1 failed attempt on refactor/range-control-to-typescript.
[2022-05-16T09:51:42.310Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-16T10:08:17.403Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-17T12:57:57.490Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-20T11:35:12.874Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-20T11:56:30.526Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-20T13:18:43.975Z] Test passed after 1 failed attempt on wp/6.0.
[2022-05-23T11:13:56.560Z] Test passed after 1 failed attempt on wp/6.0.
[2022-06-21T14:02:54.075Z] Test passed after 1 failed attempt on wp/6.0.
[2022-06-21T14:19:10.028Z] Test passed after 1 failed attempt on wp/6.0.
[2022-06-21T14:28:32.099Z] Test passed after 1 failed attempt on wp/6.0.
[2022-06-30T14:24:57.116Z] Test passed after 1 failed attempt on wp/6.0.
[2022-06-30T14:54:03.214Z] Test passed after 1 failed attempt on wp/6.0.
[2022-07-05T12:10:45.699Z] Test passed after 1 failed attempt on wp/6.0.
[2022-07-05T12:18:45.545Z] Test passed after 1 failed attempt on wp/6.0.
[2022-07-20T09:28:47.670Z] Test passed after 1 failed attempt on fix/backport-variations-schema-to-6.0.
[2022-07-20T09:53:18.417Z] Test passed after 1 failed attempt on wp/6.0.
[2022-10-17T15:04:02.663Z] Test passed after 1 failed attempt on wp/6.0.
[2022-10-17T15:39:14.827Z] Test passed after 1 failed attempt on wp/6.0.
[2023-02-27T18:35:20.452Z] Test passed after 1 failed attempt on try/something.
[2023-10-04T13:48:28.825Z] Test passed after 1 failed attempt on try/debounce-rich-text-on-input.
[2023-10-25T17:09:08.557Z] Test passed after 1 failed attempt on try/block-edit-lazy-loading.
Error: Snapshot comparison failed:

  <!-- wp:paragraph -->
  <p>A block</p>
  <!-- /wp:paragraph -->

  <!-- wp:paragraph -->
  <p>B block</p>
  <!-- /wp:paragraph -->

Expected: /home/runner/work/gutenberg/gutenberg/artifacts/test-results/editor-various-copy-cut-paste-Copy-cut-paste-should-copy-only-partial-selection-of-text-blocks-chromium/Copy-cut-paste-should-copy-only-partial-selection-of-text-blocks-1-expected.txt
Received: /home/runner/work/gutenberg/gutenberg/artifacts/test-results/editor-various-copy-cut-paste-Copy-cut-paste-should-copy-only-partial-selection-of-text-blocks-chromium/Copy-cut-paste-should-copy-only-partial-selection-of-text-blocks-1-actual.txt
    at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/copy-cut-paste.spec.js:260:49
@github-actions github-actions bot added the [Type] Flaky Test Auto-generated flaky test report issue label Apr 13, 2022
@kevin940726
Copy link
Member

Hi, @ntsekouras 👋. Seems like the test is pretty flaky since introduced in #40098. Do you know what might have gone wrong here?

@ntsekouras
Copy link
Contributor

Same comment with here.

I'm wondering if it's related to #39807. Locally I cannot reproduce and both flaky tests seem to fail only once and then always pass.

I could see that in the migrated util there were changes like here that didn't match the previous util function and we have an clipboardDataHolder object that is being used/shared on copy/cut/paste actions and is being mutated. I'm not sure if there is a connection, but it feels weird and needs more looking.

In general a snapshot like this:

<!-- wp:paragraph -->
  <p>A undefinedundefinedundefinedblockundefinedundefinedundefined</p>
  <!-- /wp:paragraph -->

seems really weird and makes me think there is something wrong in our custom emulateClipboard function.. I haven't pinpointed the reason yet.

@kevin940726
Copy link
Member

I think I've found the issue of this one. The part of code that causes the problem is this:

await pageUtils.pressKeyWithModifier( 'shift', 'ArrowUp' ); // (a)
// Not important: await pageUtils.pressKeyWithModifier( 'primary', 'c' );
await pageUtils.pressKeyWithModifier( 'primary', 'ArrowLeft' ); // (b)
await page.keyboard.press( 'Enter' ); // (c)
await page.keyboard.press( 'ArrowUp' ); // (d)

If all works according to plan:

  1. (a) selects a range of the content. (block\nB in particular)
  2. (b) moves the caret cursor to the start of the line. (Before A)
  3. (c) inserts a new line above the first line. (Above A block)
  4. (d) moves the caret cursor to the line above.

However, for some reason (probably because the editor code is unresponsive and blocking the UI 😞), sometimes the third step runs first and completely swallows the second step. This means the results then become:

  1. (a) selects a range of the content. (block\nB in particular)
  2. (c) replaces the selected content with a new line. (Above A block)
  3. (d) moves the caret cursor to the line above. (Before A)

I recorded a video with an intentional slowdown of the results:

Kapture.2022-04-26.at.15.01.31.mp4

The reason why it will only fail on the first run and will pass on the second retry is totally an accident. Turns out adding traces will insert some kind of intermediate steps between each action, and these steps accidentally wait for the caret to move to the correct position before inserting the new line. This means #40586 will probably accidentally fix this issue.

It's also the reason why it's so hard to reproduce it locally now because traces are always being recorded when run in local. I can reproduce this issue if I go to playwright.config.ts and comment out the trace: ... line.

However, if we want to make the fix explicit, we would probably want to do one of the following:

  1. Track down why it only happens on the block editor and fix it there. Is something in our app running too slow?
  2. Add an additional check before performing await page.keyboard.press( 'Enter' ). I tested with await page.waitForFunction( () => window.getSelection().type === 'Caret' ) and it works perfectly locally.

I think we should go with the second one for now. I don't have enough knowledge if the first one is even a legitimate bug or not.

I didn't debug #40307 though, but I guess it might be something similar?

@talldan
Copy link
Contributor

talldan commented Apr 26, 2022

Nice work tracking down what was happening.

Add an additional check before performing await page.keyboard.press( 'Enter' ). I tested with await page.waitForFunction( () => window.getSelection().type === 'Caret' ) and it works perfectly locally.

That sounds like a plan 👍 .

A code comment could be added above this line of code linking to this issue comment.

@ntsekouras
Copy link
Contributor

Good findings @kevin940726!

I'll open a quick PR with the suggestion to await for the caret for now, thanks!

@kevin940726
Copy link
Member

It's probably reopened because of some outdated branch. Let's close it again.

@github-actions github-actions bot reopened this Apr 27, 2022
@github-actions github-actions bot reopened this May 16, 2022
@github-actions
Copy link
Author

This issue has gone 30 days without any activity.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Aug 25, 2022
@github-actions github-actions bot reopened this Oct 17, 2022
@github-actions github-actions bot removed the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Oct 18, 2022
@github-actions
Copy link
Author

This issue has gone 30 days without any activity.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Nov 18, 2022
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2022
@github-actions github-actions bot reopened this Feb 27, 2023
@github-actions github-actions bot mentioned this issue Feb 27, 2023
@Mamaduka Mamaduka closed this as completed Mar 2, 2023
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 22, 2023
@github-actions github-actions bot reopened this Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Flaky Test Auto-generated flaky test report issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants