Random fail in \Drupal\Tests\views\FunctionalJavascript\Plugin\views\Handler\ContextualFilterTest::testAddContextualFilterUI

see
https://www.drupal.org/pift-ci-job/646538
https://www.drupal.org/pift-ci-job/650091
https://www.drupal.org/pift-ci-job/655859
https://www.drupal.org/pift-ci-job/729921

It contains 6 assertWaitOnAjaxRequest() calls, so those probably need to be taken out and replaced by some waitForElement calls or something

Occurrence rate seems to be pretty low.

CommentFileSizeAuthor
#18 x500_2868880-16.patch3.77 KBAnonymous (not verified)
#16 2868880-16.patch3.17 KBLendude
#16 interdiff-2868880-6-16.patch522 bytesLendude
#12 2868880-baseline-400x.patch622 bytesmichielnugter
#12 2868880-baseline-300x.patch622 bytesmichielnugter
#10 2868880-baseline-200x.patch622 bytesLendude
#9 2868880-baseline-100x.patch622 bytesLendude
#6 2868880-6.patch2.99 KBmichielnugter
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lendude created an issue. See original summary.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
Parent issue: » #2829040: [meta] Known intermittent, random, and environment-specific test failures
droplet’s picture

droplet CreditAttribution: droplet commented
Issue tags: +js-novice, +Novice
Related issues: +#2856047: Avoid random failures in JavascriptTestBase when testing functionality in a dialog

It was a bad style (not recommended) of coding.

assertWaitOnAjaxRequest should be changed to waitForElementVisible (best), or waitForElement.

    // Check that the dialog opens.
    $web_assert->assertWaitOnAjaxRequest();
    $page->pressButton('Close');

@see: #2856047: Avoid random failures in JavascriptTestBase when testing functionality in a dialog

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Status: Active » Postponed

Checked out the test real quick and I think the problem here is with the dialog and resizing again. We might need to postpone this one on #2856047: Avoid random failures in JavascriptTestBase when testing functionality in a dialog.

EDIT 1: After that get's in we should definitely improve this test on used waiting methods.

EDIT 2: To clarify: wait methods alone are not enough, the buttons will sometimes be visible but move around due to resize events after the visible check passes.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
Issue summary: View changes

added another failure instance (thanks @mpdonadio)

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Status: Postponed » Needs review
FileSize
2868880-6.patch2.99 KB

Looked into it a little deeper and found out that it wasn't the dialog that was playing up but it actually was the unreliable assertWaitOnAjaxRequests that we're causing problems.

I converted all to a assertWaitOnAjaxRequest calls to alternatives. I also added the disable for the live preview to prevent errors on saving the view, the button triggered a cancel on the preview ajax request which in turn caused javascript errors.

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Issue tags: -js-novice, -Novice

Random fails in Javascript tests are never novice I think, removing the tags.

droplet’s picture

droplet CreditAttribution: droplet commented

All these Views UI JSTests have the same problem. here's one more: https://www.drupal.org/pift-ci-job/661914

Should we sort it in one thread? It should share same code patterns.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
FileSize
2868880-baseline-100x.patch622 bytes

@droplet I like the idea but we should really try to establish a baseline fail rate per test so we are sure that after fixing the test it is really fixed (so hopefully we don't have to go back to it 3 times like this #2870146: Even more random fails in \Drupal\Tests\outside_in\FunctionalJavascript\OutsideInBlockFormTest).

I definitely think we should open up issues for all views ui tests to update them and not wait until somebody opens a 'random fail' issue for it, since we are pretty sure they will fail at some point.

Lets establish a baseline fail rate for this test, first attempt at 100 runs.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
FileSize
2868880-baseline-200x.patch622 bytes

Lets crank it up to 200, that should still work.

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented

Added slow and fast configurations for tests to see what that does, might we get a fail this time?

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
FileSize
2868880-baseline-300x.patch622 bytes
2868880-baseline-400x.patch622 bytes

300 and 400 times to see how they hold up.

If this doesn't work I'm going to see if I can trigger the fail some other way..

The last submitted patch, 12: 2868880-baseline-300x.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 12: 2868880-baseline-400x.patch, failed testing.

droplet’s picture

droplet CreditAttribution: droplet commented

Obviously, we fixing the tests, not a non-test bug. Now the failures are predictable basically. Patch like #6 is a good refactoring. In whatever way, we should get it in.

The diff code pattern has diff fail rate. Why not running the patch version 200x ~ 2000x instead and then kick it in and let the wild bots testing it? We need not figure out outdated code failure rate. (Remember that it's never the final code, we will do many code refactoring / additional in the future features...etc)

The only bad thing is our d.org credits increasing so fast :P

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
Status: Needs work » Needs review
FileSize
interdiff-2868880-6-16.patch522 bytes
2868880-16.patch3.17 KB

This is playing up again:

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

Tiny tweak to the #6 patch, and agree with @droplet lets not worry about fail rates and just get this in.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.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
x500_2868880-16.patch3.77 KB

#12 confirms a random fail. #16 looks logical. But let's check it before RTBC.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

4000 passes (#12 fell twice on this number). So, RTBC with #16. Thanks!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!

  • catch committed ad36d81 on 8.5.x 
    Issue #2868880 by Lendude, michielnugter, vaplas: Random fails in...

  • catch committed e5e9d52 on 8.4.x 
    Issue #2868880 by Lendude, michielnugter, vaplas: Random fails in...

Status: Fixed » Closed (fixed)

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

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue tags: +Random test failure
Parent issue: #2829040: [meta] Known intermittent, random, and environment-specific test failures »