Problem/Motivation

(At least) Since olivero became the new default theme in the standard profile in #3219959: Update standard profile so Olivero is the default theme, we're seeing a (dramatic) increase of test-failures in \Drupal\Tests\quickedit\FunctionalJavascript\SettingsTrayIntegrationTest like these:

1) Drupal\Tests\quickedit\FunctionalJavascript\SettingsTrayIntegrationTest::testQuickEditLinks
WebDriver\Exception\UnknownError: element click intercepted: Element ... is not clickable at point (504, 188). Other element would receive the click: ...
  (Session info: headless chrome=98.0.4758.102)
  (Driver info: chromedriver=98.0.4758.102 (273bf7ac8c909cde36982d27f66f3c70846a3718-refs/branch-heads/4758@{#1151}),platform=Linux 4.9.0-0.bpo.6-amd64 x86_64)

/var/www/html/vendor/instaclick/php-webdriver/lib/WebDriver/Exception.php:156
/var/www/html/vendor/instaclick/php-webdriver/lib/WebDriver/AbstractWebDriver.php:165
/var/www/html/vendor/instaclick/php-webdriver/lib/WebDriver/AbstractWebDriver.php:245
/var/www/html/vendor/instaclick/php-webdriver/lib/WebDriver/Container.php:239
/var/www/html/vendor/behat/mink-selenium2-driver/src/Selenium2Driver.php:806
/var/www/html/vendor/behat/mink-selenium2-driver/src/Selenium2Driver.php:792
/var/www/html/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php:146
/var/www/html/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php:205
/var/www/html/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php:157
/var/www/html/vendor/behat/mink/src/Element/NodeElement.php:153
/var/www/html/core/modules/settings_tray/tests/src/FunctionalJavascript/SettingsTrayTestBase.php:54
/var/www/html/core/modules/quickedit/tests/src/FunctionalJavascript/SettingsTrayIntegrationTest.php:117
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:726

Why?

Over on #3219959-83: Update standard profile so Olivero is the default theme I've created a patch that shows a screenshot of (what I think is) the issue, attached it here as well.
The QuickEdit Toolbar (barely) covers a little bit of the "Powered By"-block. (Depending on the characters in the name of the user that was created to create the blocks, there's the randomness.)

We perform a click on that block. That's a rather big place to do a click on, somehow the click() command seems to like to click exactly on the tiny spot that is covered by the QuickEdit-toolbar. Which in turn causes the block not receiving the click like the error above states.

Proposed resolution

There's a difference in the number of blocks in the core/themes/olivero/config/optional directory between 9.4/9.5 and 10.0. In #3219959: Update standard profile so Olivero is the default theme we moved blocks for 9.4 and 9.5, whilst in 10.0 we copied them.

Looking at the screenshots I've made locally and attached here for Olivero and Bartik whilst running \Drupal\Tests\quickedit\FunctionalJavascript\SettingsTrayIntegrationTest, copying looks better and passes the test.

Let's restore the blocks in core/themes/olivero/config/optional so we have:

- the same amount of blocks between 9.4/9.5 and 10.0
- a passing \Drupal\Tests\quickedit\FunctionalJavascript\SettingsTrayIntegrationTest

Previously attempted resolution

Move the block that is tested to be the bottom block on the page. The QuickEdit Toolbar is always _above_ the field that is being edited. If our block is below it, it will never be covered.

Issue fork drupal-3278215

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Spokje created an issue. See original summary.

spokje’s picture

eojthebrave’s picture

We fixed another section of this test when Olivero became the default theme. Here https://www.drupal.org/project/drupal/issues/3219959#comment-14498697

I'm not sure if it's related or not, but in case it is. Line 118 of the current test is:

// QuickEdit toolbar should be closed when opening Off-canvas dialog.
$web_assert->elementNotExists('css', $quick_edit_selector);

Which is almost identical to the lines we changed just below that to:

// QuickEdit toolbar should be closed when opening off-canvas dialog.
$web_assert->waitForElementRemoved('css', $quick_edit_selector);
spokje’s picture

Issue summary: View changes
StatusFileSize
new32.09 KB
spokje’s picture

spokje’s picture

Assigned: spokje » Unassigned
Status: Active » Needs review
spokje’s picture

Issue summary: View changes

Note: The patches change core/drupalci.yml, please use the MR for committing (if it comes to that).

spokje’s picture

Title: (Not so) Random failure SettingsTrayIntegrationTest » (Not so) Random test failures SettingsTrayIntegrationTest

spokje’s picture

9.3.x-dev is not affected, since [##3219959] was only committed to 9.4, 9.5 and 10.0.
No clue why 10.0.x-dev is not showing the test failures, but then again: The fix also doesn't show test-failures, so it isn't breaking anything there.

spokje’s picture

Status: Needs review » Postponed
Related issues: +#3278342: Theming blocks bonanza

Let's postpone this on #3278342: Theming blocks bonanza, that _might_ solve the root cause, instead of fighting the fallout in here.

spokje’s picture

Status: Postponed » Needs review

Back to NR, since #3278342: Theming blocks bonanza won't make it (at least for now).

spokje’s picture

Assigned: Unassigned » spokje
Issue summary: View changes
Status: Needs review » Needs work
spokje’s picture

StatusFileSize
new41.88 KB
new22.17 KB
new42.27 KB
new31.33 KB

spokje’s picture

Let's give the ol' "20-times-break" and "20-times-fix" approach another go with the new proposed solution.

No interdiff, because it's a new approach.

spokje’s picture

Issue summary: View changes
spokje’s picture

Issue summary: View changes
spokje’s picture

Assigned: spokje » Unassigned
Status: Needs work » Needs review
spokje’s picture

lauriii’s picture

Priority: Normal » Critical

Marking as critical given the impact of this issue for the stability of the CI tests for 9.4.x and 9.5.x.

spokje’s picture

Assigned: Unassigned » spokje
Priority: Critical » Normal
Status: Needs review » Needs work
spokje’s picture

Priority: Normal » Critical

spokje’s picture

Assigned: spokje » Unassigned
Status: Needs work » Needs review

D9.4/9.5: MR!2217
D10.0: MR!2220

berdir’s picture

This feels like a somewhat strange fix that has a far bigger impact on everything, not just this specific test? Having the blocks in the theme means that all tests and also all real sites, new installations not using standard and existing ones will get these blocks by default.

Could we instead place the block that's added in the test in a different way? give that a negative weight, so it's above the conflicting block?

Any chance this is somehow also related to the default-region change in 9.4 that was done for olivero as it doesn't have sidebar_first? this caused a lot of problems in paragraphs.module.

lauriii’s picture

This change will indeed help address the random failure in the SettingsTrayIntegrationTest, but it will do more than that. I think it's expected with Olivero gets certain blocks placed because it is a much more opinionated theme than Bartik. I think we are currently assuming that any install profile using plain Olivero, would want a similar block placement to what Olivero ships with.

I'd also like to note that this is simply taking this back to where it was before #3219959: Update standard profile so Olivero is the default theme. If we want to provide additional flexibility on this, we could do that in a follow-up issue.

lauriii’s picture

Status: Needs review » Reviewed & tested by the community

Opened an issue for discussing #27: #3278674: Decide how blocks should be shipped for Claro and Olivero. Berdir confirmed on Slack that he would be fine with discussing this in a follow-up.

I've tested both MR's manually and reviewed that all of the block configurations after the MR's are now in Olivero config/optional.

catch’s picture

Status: Reviewed & tested by the community » Fixed

We discussed this and related issue at length in slack.

The original weight fix in here would resolve the specific random test failure (more like random test pass) in this issue, but all the inconsistencies between Claro/Olivero/9.4.x/10.0.x blocks are likely to keep biting us. So getting everything aligned and then going from there is good.

Committed/pushed the 10.0.x MR to 10.0.x. Committed/pushed to 9.5.x MR and cherry-picked to 9.4.x, thanks!

  • catch committed eed84cd on 10.0.x
    Issue #3278215 by Spokje, lauriii, eojthebrave, Berdir: (Not so) Random...
  • catch committed 2b2203f on 9.4.x
    Issue #3278215 by Spokje, lauriii, eojthebrave, Berdir: (Not so) Random...
  • catch committed 20e0d59 on 9.5.x
    Issue #3278215 by Spokje, lauriii, eojthebrave, Berdir: (Not so) Random...

Status: Fixed » Closed (fixed)

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