Problem/Motivation

https://www.drupal.org/pift-ci-job/849736

Drupal\Tests\system\FunctionalJavascript\ThemeFormSettingsTest::testFormSettingsSubmissionHandler with data set "test theme.theme" ('test_theme_theme')
Failed asserting that false is true.

Proposed resolution

Check if it just unstable work from $page->pressButton();, because we already have very similar case in #2924201: Resolve random failure in LayoutBuilderTest so that it can be added to HEAD

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#15 interdiff-12-15.txt803 bytesAnonymous (not verified)
#15 2934064-15-web-drive.patch1.91 KBAnonymous (not verified)
#14 assertWaitOnClick.patch3.08 KBAnonymous (not verified)
#12 interdiff-11-12.txt745 bytesAnonymous (not verified)
#12 2934064-12-test-only.patch1.75 KBAnonymous (not verified)
#2 x1000-ThemeFormSettingsTest.patch555 bytesAnonymous (not verified)
#2 x100-f2000-ThemeFormSettingsTest-mod1.patch1.81 KBAnonymous (not verified)
#3 x100-f2000-ThemeFormSettingsTest-mod2.patch1.71 KBAnonymous (not verified)
#5 x500-f500-PressButtonTest.patch1.63 KBAnonymous (not verified)
#6 x10-f1000-PressButtonTest.patch1.63 KBAnonymous (not verified)
#8 x10-f1000-PressButtonTest-fix-2934997-27.patch1.86 KBAnonymous (not verified)
#8 interdiff-fix-from-2934997-27.txt1.16 KBAnonymous (not verified)
#10 2934064-10.patch2.72 KBAnonymous (not verified)
#11 2934064-11.patch1.82 KBAnonymous (not verified)
#11 interdiff-8-11.txt1.14 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

vaplas created an issue. See original summary.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
x1000-ThemeFormSettingsTest.patch555 bytes
x100-f2000-ThemeFormSettingsTest-mod1.patch1.81 KB
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Active » Needs review
FileSize
x100-f2000-ThemeFormSettingsTest-mod2.patch1.71 KB

#2: bad patches, sorry.

And yep, with current patch i catch random fail even localy (after 100-500 circles). So this is definitely a flaw in the pressButton() method.

Edit:
Yep, Mr. Bot also confirmed it:

https://dispatcher.drupalci.org/job/drupal_patches/42607/console

17:44:32 Drupal\Tests\system\FunctionalJavascript\ThemeFormSettingsTe   2 passes                                      
18:16:26 Drupal\Tests\system\FunctionalJavascript\ThemeFormSettingsTe   0 passes   1 fails

https://dispatcher.drupalci.org/job/drupal_patches/42609/console

17:33:16 Drupal\Tests\system\FunctionalJavascript\ThemeFormSettingsTe   2 passes                                      
17:58:00 Drupal\Tests\system\FunctionalJavascript\ThemeFormSettingsTe   2 passes                                      
18:22:47 Drupal\Tests\system\FunctionalJavascript\ThemeFormSettingsTe   2 passes                                      
18:41:27 Drupal\Tests\system\FunctionalJavascript\ThemeFormSettingsTe   0 passes   1 fails

Status: Needs review » Needs work

The last submitted patch, 3: x100-f2000-ThemeFormSettingsTest-mod2.patch, failed testing. View results

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Title: Random fail in ThemeFormSettingsTest » Random fail in JS-tests due to unstable pressButton()
FileSize
x500-f500-PressButtonTest.patch1.63 KB

Edit: Detected random fail in

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
x10-f1000-PressButtonTest.patch1.63 KB

Edit: finally met the time limit :)

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
x10-f1000-PressButtonTest-fix-2934997-27.patch1.86 KB
interdiff-fix-from-2934997-27.txt1.16 KB

Test fix from #2934997-27: Intermittent failure in MediaUiJavascriptTest.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
CreditAttribution: benjifisher at Isovera commented

@vaplas:

I am not sure what you are trying to do here. In the patch from #8, I see this:

+      $page->pressButton('Save configuration');
+      $this->assertContains('The configuration options have been saved.', $page->getContent(), "Dectected #$i");

Based on the results from #2934997: Intermittent failure in MediaUiJavascriptTest, I would expect that to fail (intermittently). If you want to test the fix from that issue, then you need something like this:

+      $page->pressButton('Save configuration');
+      $result = $assert_session->waitForElement('css', '.messages--status h2');
+      $this->assertNotEmpty($result);
+      $this->assertContains('The configuration options have been saved.', $page->getContent(), "Dectected #$i");

That is: after pressing the button, wait for the status message to appear and then assert that the text is on the page.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
2934064-10.patch2.72 KB

@benjifisher, thank you for review and hint. Let's check #9.

Also my test is terribly clogging the disk place, because of logging all pages. So, replaced it with manual recording in problem cases.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
interdiff-8-11.txt1.14 KB
2934064-11.patch1.82 KB

#9: @benjifisher, wow! Looks like your hint works! (And my attempt to eliminate logging, no. Try again.)

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
2934064-12-test-only.patch1.75 KB
interdiff-11-12.txt745 bytes

#11: Now without logs. Ok. Here is #6 version without logs too. In order to ensure that without waiting we still have a random fail.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented 
+++ b/core/tests/Drupal/FunctionalJavascriptTests/Tests/PressButtonTest.php
@@ -0,0 +1,39 @@
+      $assert_session->waitForElement('css', '.messages--status h2');

This is coupling the test to Classy theme :\

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Related issues: +#2936122: Find out why JavscriptTestBase occasionally needs a waitForElement on a normal page load, +#2936432: Review use of pressButton() in functional javascript tests in the Media module, +#2773791: Clicking elements with children in Javascript tests throws a GastonJS exception, +#2789381: Submitting an ajax dialog form in Javascript test throws a GastonJS error
FileSize
assertWaitOnClick.patch3.08 KB

#13: Indeed. But the message does not even always appear after clicking/pressing (pressButton(), press(), clickLink(), click()). So we can not use this sign for all cases.

We have issue #2936122: Find out why JavscriptTestBase occasionally needs a waitForElement on a normal page load for to investigate this problem. Until we find the cause, we can provide a spec assertion for this cases. Example assertWaitOnClick (like assertWaitOnAjaxRequest).

Or wait JSB enhancements (maybe the problem will go away by itself):

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
2934064-15-web-drive.patch1.91 KB
interdiff-12-15.txt803 bytes
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

#2775653: JavascriptTests with webDriver Rocks! All runs in #15 are green (just some of them lacked a time limit) 🎉

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs work » Closed (duplicate)

Closed in favor of #2936122: Find out why JavscriptTestBase occasionally needs a waitForElement on a normal page load. Thanks to everyone who participated here!