\Drupal\Tests\settings_tray\FunctionalJavascript\SettingsTrayBlockFormTest::testBlocks() is failing because the \Drupal\Tests\settings_tray\FunctionalJavascript\SettingsTrayTestBase::openBlockForm() is failing to open the form. If you put a sleep in here...
protected function openBlockForm($block_selector, $contextual_link_container = '') {
if (!$contextual_link_container) {
$contextual_link_container = $block_selector;
}
// Ensure that contextual link element is present because this is required
// to open the off-canvas dialog in edit mode.
$contextual_link = $this->assertSession()->waitForElement('css', "$contextual_link_container .contextual-links a");
$this->assertNotEmpty($contextual_link);
// When page first loads Edit Mode is not triggered until first contextual
// link is added.
$this->assertNotEmpty($this->assertSession()->waitForElementVisible('css', '.dialog-off-canvas-main-canvas.js-settings-tray-edit-mode'));
// THIS SLEEP MAKES IT ALWAYS PASS...
sleep(1);
$block = $this->getSession()->getPage()->find('css', $block_selector);
$block->mouseOver();
$block->click();
$this->waitForOffCanvasToOpen();
$this->assertOffCanvasBlockFormIsValid();
}
Prior to #3316274: Stabilize FunctionalJavascript testing AJAX: add ::assertExpectedAjaxRequest() there was
// Ensure that all other Ajax activity is completed.
$this->assertSession()->assertWaitOnAjaxRequest();
Where the SLEEP above is...
It'll always pass... also if you change \Drupal\Tests\settings_tray\FunctionalJavascript\SettingsTrayTestBase::getTestThemes() to return only stark it'll pass... and if you get it to return ['stark, 'stable9'] it'll fail on stable9 and not stark. All very odd.
Show commandsStart 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:
changes
Comments
Comment #2
catchComment #3
catchThe block opening isn't an AJAX request, which is why it was removed in the other issue, but it is a CSS animation or whatever, so the sleep makes sense. The wait on ajax request was making this pass randomly by at least waiting a bit, even if not for an AJAX request to complete.
Proper fix should be done in #3317520: [random test failure] Drupal\Tests\settings_tray\FunctionalJavascript\SettingsTrayBlockFormTest::testEditModeEnableDisable but I think a sleep(1) if fine for now if it makes it pass all the time.
Comment #4
alexpott@catch I think there is something else going on here... it passes if you only test stark but if you test both stark and stable9 then it'll fail on the second regardless of order.
I've tried clearing the cache after changing the theme but it made no difference...
Comment #6
alexpottGot a repeat job running on this patch - https://git.drupalcode.org/project/drupal/-/jobs/360205
...and HEAD - https://git.drupalcode.org/project/drupal/-/jobs/358222
Comment #7
larowlanWhat if we enable the css_disable_transitions_test module?
Comment #8
alexpott@larowlan thanks for trying! For others following along we already enable the css_disable_transitions_test module in \Drupal\FunctionalJavascriptTests\WebDriverTestBase::installModulesFromClassProperty()
Comment #9
alexpottSo this gets odder... it is passing on HEAD when run 100 times - yet on other MRs it is failing lots - see https://git.drupalcode.org/project/drupal/-/jobs/360407 and it fails locally.
So that leaves me to pndering what at the differences between MR tests and prod tests and do they get different resources somehow.
Comment #10
catchFor at least some tests I assume we get issues due to resource usage during a particular test, which when it runs at the same time as a test that's timing dependent, then causes that test to fail. This would explain why running the same test 100 times is different to running it amongst other tests, but it's not really based on anything concrete.
Comment #11
catchNote that
testEditModeEnableDisable()in the same test was skipped recently due to random failures, these particular random failures started happening after that was skipped I think, then even more after the ajax wait issue.Comment #12
catchThis passes locally for me...
Pushed a commit which still passes locally for me, but maybe it makes a difference elsewhere - just switching to the new assertWaitOnAjaxRequest() in OffCanvasTestBase.
Comment #14
spokjeAbsolutely unsure if this is the root cause, but it certainly doesn't help that pressing the 'Save Site branding' in the overlay, when using any theme, returns you to the (non-existing) URL
/2, instead of expected/user/2...This happens at line 122 in this test.
EDIT: Not the root cause according to my local failures, but it seems like a real bug, opened #3402650: Incorrect return from \Drupal\settings_tray\Block\BlockEntitySettingTrayForm::getRedirectUrl possible
EDIT2: Hmmm, this fails for me locally 100%, but not on GitLab CI: https://git.drupalcode.org/project/drupal/-/merge_requests/5468/diffs
Solution locally is https://git.drupalcode.org/project/drupal/-/merge_requests/5468/diffs?co..., since
$destinationlocally already is prepended with a `\`, adding an extra one makes the return URL incorrect.Anyone else sees the incorrect URL locally?
Comment #15
catchIf the sleep(1) causes this to pass, I think we should commit that workaround to stop the bleeding - it's a more honest situation than the previous, fake, waitForAjaxRequest() call. Then we can continue in #3317520: [random test failure] Drupal\Tests\settings_tray\FunctionalJavascript\SettingsTrayBlockFormTest::testEditModeEnableDisable.
Comment #16
alexpott100ms sleep fixes this 100% pf the time for me.
I've updated the MR to have an @todo to https://www.drupal.org/project/drupal/issues/3317520 and remove the assertion that @catch added that broke.
The problem is happening because we're clicking before the click handler that's added in core/modules/settings_tray/js/settings_tray.js:207 - I have no idea why it's the second theme that causes this 100% of the time locally for me.
I think the MR is committable.
I will add more detail about my findings to #3317520: [random test failure] Drupal\Tests\settings_tray\FunctionalJavascript\SettingsTrayBlockFormTest::testEditModeEnableDisable
Comment #20
lauriiiCommitting from needs review because it seems that this is now failing almost 100% of the time on Gitlab CI too.
Committed 4f47df3 and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!
Comment #21
alexpott