[random test failure] Try to un-skip and fix InlineBlockTest::testDeletion in context of [#3353085] [#3353100]

Problem/Motivation

In #3350972: [random test failure] Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderUiTest::testReloadWithNoSections() @nod_ introduced a "fix" for problems with the correct opening of an off-canvas dialog which potentially could fix all JS random test failures for tests using the drupal.dialog.off_canvas somehow.

The basic test is:

Unskip \Drupal\Tests\layout_builder\FunctionalJavascript\InlineBlockTest::testDeletion.
Run _only_ now unskipped \Drupal\Tests\layout_builder\FunctionalJavascript\InlineBlockTest::testDeletion a lot of times (~~usually we go for 1500x~~) as-is.
Run _only_ now unskipped \Drupal\Tests\layout_builder\FunctionalJavascript\InlineBlockTest::testDeletion a lot of times (~~usually we go for 1500x~~) _without_ the changes in #3350972: [random test failure] Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderUiTest::testReloadWithNoSections().
If 2., passes and 3. doesn't, we can safely turn the test back on again.

Per @xjm in #3353085-4: [meta] Determine impact of [#3350972] fix in off-canvas.js on currently disabled FunctionalJavascript tests we now need to go for a 5000-8000 times run for both patches.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comment	File	Size	Author
#6	3353100-6.patch	811 bytes	spokje
#5	InlineBlockTest_testDeletion-no_#3350972-1500x-should_fail.patch	24.72 KB	spokje
#5	InlineBlockTest_testDeletion-1500x.patch	24.23 KB	spokje
#4	InlineBlockTest_testDeletion-no_#3350972-1500x-should_fail.patch	11.8 KB	spokje
#4	InlineBlockTest_testDeletion-1500x.patch	11.3 KB	spokje
#3	InlineBlockTest_testDeletion-no_#3350972-1000x-should_fail.patch	11.75 KB	spokje
#3	InlineBlockTest_testDeletion-1000x.patch	11.25 KB	spokje
#2	InlineBlockTest_testDeletion-no_#3350972-1500x-should_fail.patch	11.75 KB	spokje
#2	InlineBlockTest_testDeletion-1500x.patch	11.25 KB	spokje

Comments

Comment #1

9 April 2023 at 13:26

Spokje created an issue. See original summary.

Comment #2

spokje

Dutch

commented 9 April 2023 at 13:26

Status	File	Size
new	InlineBlockTest_testDeletion-1500x.patch	11.25 KB
new	InlineBlockTest_testDeletion-no_#3350972-1500x-should_fail.patch	11.75 KB

Comment #3

spokje

Dutch

commented 9 April 2023 at 14:55

Status	File	Size
new	InlineBlockTest_testDeletion-1000x.patch	11.25 KB
new	InlineBlockTest_testDeletion-no_#3350972-1000x-should_fail.patch	11.75 KB

2 files were hidden/shown/deleted

Status	File	Size
hidden	InlineBlockTest_testDeletion-1500x.patch	11.25 KB
hidden	InlineBlockTest_testDeletion-no_#3350972-1500x-should_fail.patch	11.75 KB

Test seems too slow for 1500x, let's try 1000x.

Comment #4

spokje

Dutch

commented 9 April 2023 at 16:13

Status	File	Size
new	InlineBlockTest_testDeletion-1500x.patch	11.3 KB
new	InlineBlockTest_testDeletion-no_#3350972-1500x-should_fail.patch	11.8 KB

2 files were hidden/shown/deleted

Status	File	Size
hidden	InlineBlockTest_testDeletion-1000x.patch	11.25 KB
hidden	InlineBlockTest_testDeletion-no_#3350972-1000x-should_fail.patch	11.75 KB

Right...

Let's do what I've should have done from the start, test _only_ \Drupal\Tests\layout_builder\FunctionalJavascript\InlineBlockTest::testDeletion

Comment #5

spokje

Dutch

commented 9 April 2023 at 16:30

Status	File	Size
new	InlineBlockTest_testDeletion-1500x.patch	24.23 KB
new	InlineBlockTest_testDeletion-no_#3350972-1500x-should_fail.patch	24.72 KB

2 files were hidden/shown/deleted

Status	File	Size
hidden	InlineBlockTest_testDeletion-1500x.patch	11.3 KB
hidden	InlineBlockTest_testDeletion-no_#3350972-1500x-should_fail.patch	11.8 KB

Comment #6

spokje

Dutch

commented 9 April 2023 at 17:09

Status:

Active

» Needs review

Status	File	Size
new	3353100-6.patch	811 bytes

2 files were hidden/shown/deleted

Status	File	Size
hidden	InlineBlockTest_testDeletion-1500x.patch	24.23 KB
hidden	InlineBlockTest_testDeletion-no_#3350972-1500x-should_fail.patch	24.72 KB

Very minimal difference, only one fail in the should_fail, but it is with

1) Drupal\Tests\layout_builder\FunctionalJavascript\InlineBlockTest::testDeletion
Error: cannot call methods on dialog prior to initialization; attempted to call method 'widget'
    at Function.error (http://php-apache-jenkins-drupal-patches-177329/subdirectory/core/assets/vendor/jquery/jquery.min.js?v=3.6.3:2:2616)
    at HTMLDivElement. (http://php-apache-jenkins-drupal-patches-177329/subdirectory/core/assets/vendor/jquery.ui/ui/widget-min.js?v=10.1.0-dev:9:2224)
    at Function.each (http://php-apache-jenkins-drupal-patches-177329/subdirectory/core/assets/vendor/jquery/jquery.min.js?v=3.6.3:2:3003)
    at E.fn.init.each (http://php-apache-jenkins-drupal-patches-177329/subdirectory/core/assets/vendor/jquery/jquery.min.js?v=3.6.3:2:1481)
    at t.fn. [as dialog] (http://php-apache-jenkins-drupal-patches-177329/subdirectory/core/assets/vendor/jquery.ui/ui/widget-min.js?v=10.1.0-dev:9:1962)
    at Object.getContainer (http://php-apache-jenkins-drupal-patches-177329/subdirectory/core/misc/dialog/off-canvas/js/off-canvas.js?v=10.1.0-dev:302:23)
    at resetSize (http://php-apache-jenkins-drupal-patches-177329/subdirectory/core/misc/dialog/off-canvas/js/off-canvas.js?v=10.1.0-dev:207:42)
    at later (http://php-apache-jenkins-drupal-patches-177329/subdirectory/core/misc/debounce.js?v=10.1.0-dev:37:23)

which is the exact failure that was fixed in #3350972: [random test failure] Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderUiTest::testReloadWithNoSections(), so I believe this one to be fixed as well.

Comment #7

catch

he/him

English

commented 10 April 2023 at 07:50

Status:

Needs review

» Reviewed & tested by the community

Amazing.

Comment #8

spokje

Dutch

commented 10 April 2023 at 09:21

Issue summary:

View changes

Comment #9

spokje

Dutch

commented 10 April 2023 at 10:36

Status:

Reviewed & tested by the community

» Needs work

Comment #10

spokje

Dutch

commented 10 April 2023 at 10:36

Assigned:

Unassigned

» spokje

Comment #11

spokje

Dutch

commented 10 April 2023 at 12:14

Assigned:	spokje	» Unassigned
Status:	Needs work	» Reviewed & tested by the community

Both should_fail and non_fail patch ran 5 times. That's 7500 individual tests per patch.

Back to RTBC.

Comment #12

catch

he/him

English

commented 10 April 2023 at 12:20

I've just committed #3353096: [random test failure] Try to un-skip and fix InlineBlockPrivateFilesTest::testPrivateFiles() in context of [#3353085], will aim to commit this one (or another similar RTBC one) in roughly 24 hours.

Comment #13

xjm

she/her

English

commented 10 April 2023 at 16:15

Our fail patch is not failing here. Queueing more jobs to see if it's just unlucky or a lower fail rate. If it continues to pass we should double-check that we're fixing the right thing, or look to external factors like the AWS instances.

Comment #14

xjm

she/her

English

commented 10 April 2023 at 16:30

Blergh, fixing attribution.

Comment #15

xjm

she/her

English

commented 10 April 2023 at 17:11

One of the should-fail-jobs in #5 now shows the expected fail, so we are good there. See what I mean about needing more test runs. :)

Comment #16

spokje

Dutch

commented 10 April 2023 at 18:33

To be fair, it did already fail: https://www.drupal.org/pift-ci-job/2639266, but more tests are better tests.

Comment #17

xjm

she/her

English

commented 10 April 2023 at 18:51

Based on the current fail rate, even 24,000 jobs (16 test runs queued) would still have a 1% chance of a false pass, so queueing even more. I think this is definitely one where the race condition is worse depending on external factors like the AWS instance or such (which is also why it's important to queue the pass- and fail-patch test jobs at the same time on the same environments).

Comment #18

11 April 2023 at 11:35

catch committed a49a8f24 on 10.1.x

Issue #3353100 by Spokje, xjm: [random test failure] Try to un-skip and...

Comment #19

catch

he/him

English

commented 11 April 2023 at 11:36

Version:	10.1.x-dev	» 10.0.x-dev
Status:	Reviewed & tested by the community	» Patch (to be ported)

Patch no longer applied for some reason, but I just made the change manually. Committed/pushed to 10.1.x, thanks!

Moving to 10.0.x for backport.

Comment #20

spokje

Dutch

commented 11 April 2023 at 14:17

Status:

Patch (to be ported)

» Needs review

Patch applies to 10.0.x, 9.5.x and 9.4.x

Comment #21

smustgrave commented 11 April 2023 at 16:22

Status:

Needs review

» Reviewed & tested by the community

Seems several testing rounds happened in #6

Comment #22

catch

he/him

English

commented 17 April 2023 at 08:15

Status:

Reviewed & tested by the community

» Patch (to be ported)

Using 'to be ported' as 'need to cherry-pick this once all the related patches are in 10.1' so it's easier to tell which have already been committed vs. not.

Comment #23

24 April 2023 at 09:11

catch committed 9fffa575 on 10.0.x

Issue #3353100 by Spokje, xjm: [random test failure] Try to un-skip and...

Comment #24

24 April 2023 at 09:11

catch committed 8bb564c1 on 9.5.x

Issue #3353100 by Spokje, xjm: [random test failure] Try to un-skip and...

Comment #25

catch

he/him

English

commented 24 April 2023 at 09:11

Version:	10.0.x-dev	» 9.5.x-dev
Status:	Patch (to be ported)	» Fixed

Cherry-picked to 10.0.x and 9.5.x, thanks!

Comment #26

8 May 2023 at 09:14

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

[random test failure] Try to un-skip and fix InlineBlockTest::testDeletion in context of [#3353085]

Problem/Motivation

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

Parent issue