Avoid random failures in JavascriptTestBase when testing functionality in a dialog

Are there any other tests that can use this? I think this might have been what dawehner was getting at in #31.

I think a dedicated test for this trait/JS is what was asked for. Sounds like a good addition, currently if we were to remove the Views test, this functionality has no coverage.

Do you have an example of testing a trait?

Something like this?

Nice, that looks great.

Code looks good, we are only updating 1 test here, should we do more here of should we open a follow-up to change more tests to use this?
I think landing this first is good, so marking RTBC, not making the follow up yet till others agree that would be the right way to handle this.

Doing more in a follow-up seems good to me.

Ok, here is that follow up 📌 Convert all JavaScript tests that use dialogs to use waitForDialog() Active

The last submitted patch, 51: 2856047-51.patch, failed testing. View results →

Random failure..

Since this introduces changes intended to reduce random fails, this should have patches that customize drupalci.yml → to run a single random-fail prone test several hundred times (possibly as many times as the test runner time limit allows). Include a version with and without the changes introduced here to compare the failure rate.

I also noticed that the solution here resembles something already in core specific to off canvas. Most of the lifting is done in these files:
https://git.drupalcode.org/project/drupal/-/blob/10.1.x/core/modules/sys...
https://git.drupalcode.org/project/drupal/-/blob/10.1.x/core/modules/sys...

Perhaps the changes here can be integrated the off canvas ones, even if it's just having the traits in the same dir/namespace?

Since this introduces changes intended to reduce random fails, this should have patches that customize drupalci.yml to run a single random-fail prone test several hundred times (possibly as many times as the test runner time limit allows). Include a version with and without the changes introduced here to compare the failure rate.

Adding both those here, before we start work on (possible) integrating with the off canvas solution.

So what we've learned here is that FilterCriteriaTest is not broken (enough) to make any difference.

Not saying the approach in the patch for dialogs is wrong, just that it can't be tested with FilterCriteriaTest

MediaLibraryTest::testButton() is a recently skipped random failure that checks for a dialog, so that might be a good one to try.

Sadly also this one doesn't seem broken enough to be used for any proof.

Maybe LayoutBuilderUiTest - just seen it come up:

.Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderUiTest
✗	
Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderUiTest
fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-107.xml:
PHPUnit Test failed to complete; Error: PHPUnit 9.5.28 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderUiTest
F.S.                                                                4 / 4 (100%)

Time: 00:36.411, Memory: 4.00 MB

There was 1 failure:

1) Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderUiTest::testReloadWithNoSections
Error: cannot call methods on dialog prior to initialization; attempted to call method 'widget'
    at Function.error (http://php-apache-jenkins-drupal-

https://www.drupal.org/pift-ci-job/2634951 →

If we can't get it to fail out of 1500 runs, it suggests the recent instability of all these tests might not be related to the changes in this patch though.

If we can't get it to fail out of 1500 runs, it suggests the recent instability of all these tests might not be related to the changes in this patch though.

Even more: If we can't get MediaLibraryTest::testButton() to fail once in 1500 times when run on it's own, I fear it looks like the failure there has to do with concurrent running other tests?

Looks like it's failing pretty well :D

got a potential fix for the issue in #65 which isn't quite an issue with the timing in tests.

Just fixing tag text.

Perhaps a lead on how to reproduce this reliably: screen resolution seems to affect it a lot.

We have a huge suite of behat (selenium chromium) tests on a project at work. I wanted to change the resolution (from 1600x1200 to 1600x900) and noticed a lot of failures with modals. Traced it to the debounce (20ms) in Drupal.dialog.resetSize causing dialogs to bounce 2-3 times. The modal appears, selenium attempts to scroll in the modal to click on a link inside, but the modal repositions again before it can scroll and throws a "move target out of bounds" error because the links is out of reach. (what a nightmare to debug).
I simply reverted to 1600x1200 for now, the modal seems more stable, way less repositioning for some reason.

Does 🐛 Error: cannot call methods on dialog prior to initialization; attempted to call method 'option' Needs review solve this?