Problem/Motivation
Follow-up from #3267124: Temporarily skip failing tests.
The following JavaScript test methods are currently skipped for random fails:
Layout Builder
LayoutBuilderUiTest::assertHighlightNotExists.
Other
MoveBlockFormTest::testMoveBlock()- #3191559: [random test failure] Random test fail in EntityReferenceWidgetTest
EntityReferenceWidgetTest::testWidget()(the infamous #3066447: [random test failure] Random failures building media library form after uploading image (WidgetUploadTest))- #3351597: [random test failure] Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest
Proposed resolution
- File children to debug and un-skip each.
- Un-skip one method per issue and run the affected test 500x.
- Queue 8 test runs of 1000x against MySQL or MariaDB environments.
- Post a patch that un-skips the test without any attempted fix to get a baseline fail rate, and always test this patch at the same time as any proposed fix with the same environment.
- Skip other methods in the test if it becomes necessary due to out-of-scope fails.
Remaining tasks
- Add items to the above summary for::
Already fixed
\Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest::testButton: #3268368: Robustify and restore \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest::testButtonLayoutBuilderDisableInteractionsTest::testFormsLinksDisabled(): #3268680: [random test failure] Restore and fix LayoutBuilderDisableInteractionsTest::testFormsLinksDisabled()- #3272797: [random test failure] Restore LayoutBuilderTest::testConfigurableLayoutSections()
QuickEditIntegrationTest::testArticleNode()#3268244: [random test failure] Un-skip and fix QuickEditIntegrationTest::testArticleNode()QuickEditIntegrationTest::testCustomBlock()#3267258: Remove Quick Edit support from editor.moduleAjaxBlockTest::testAddAjaxBlock()#3304371: Fix intermittent failure in AjaxBlockTestContentPreviewToggleTest::testContentPreviewToggle(): #3268678: [random test failure] Restore ContentPreviewToggleTest::testContentPreviewToggle()SettingsTrayBlockFormTest::testEditModeEnableDisable(): #3304901: Fix intermittently failing Settings Tray Functional Javascript testsLayoutBuilderDisableInteractionsTest::assertContextualLinksClickable()#3353103: [no random test failure] Try to un-skip and fix LayoutBuilderDisableInteractionsTest::assertContextualLinksClickable in context of [#3353085]- #3353179: [random test failure] Try to un-skip and fix LayoutBuilderTest::testLayoutBuilderUi in context of [#3353085]
- #3353153: [random test failure] Try to un-skip and fix LayoutBuilderNestedFormUiTest::testAddingFormBlocksToDefaults in context of [#3353085]
- #3353092: [random test failure] Try to un-skip and fix ContextualLinksTest in context of [#3353085]
- #3353167: [random test failure] Try to un-skip and fix LayoutBuilderNestedFormUiTest::testAddingFormBlocksToOverrides in context of [#3353085]
BlockFormMessagesTest::testValidationMessage()#3353088: [random test failure] Try to un-skip and fix BlockFormMessagesTest::testValidationMessage() in context of [#3353085]- #3351494: Skip Drupal\Tests\media\FunctionalJavascript\MediaSourceFileTest
| Comment | File | Size | Author |
|---|
Comments
Comment #2
longwaveChromedriver is updated, so let's debug the tests skipped in:
#3267124: Temporarily skip failing tests
#3267823: \Drupal\Tests\quickedit\FunctionalJavascript\QuickEditIntegrationTest::testCustomBlock(). is failing on latest chromedriver
Comment #3
longwaveComment #5
longwaveSo of the five tests here, repeated 50x each:
passed 50 times
passed 48 times, failed twice
passed 50 times
passed 50 times
failed 43 times, passed 7 times
Comment #6
longwaveContentPreviewToggleTest has two different stack traces for the two fails:
vs
QuickEditIntegrationTest mostly fails with:
but once with
Comment #7
xjmLet's try a few more runs of the passing ones to prove that they're passing.
Comment #8
longwaveThe majority of QuickEditIntegrationTest fails might be something to do with the hover state; the field name is only shown in the toolbar when the pointer is hovering over the field itself; otherwise only the entity title is shown. I am not sure where the pointer is placed in tests by default, nor do I see anything that explicitly places the pointer in these tests.
Having said that, the test does not appear to use any random factors so the result should (in theory) be deterministic? Maybe this could be a race condition though, where the hover event does not always fire?
Comment #9
xjm@longwave Yeah a good 75% or more of our random fails in JS functional tests historically are due to race conditions and not using the correct APIs to wait for elements to be rendered.
Comment #11
xjmThe other three are definitely still failing plenty:
https://dispatcher.drupalci.org/job/drupal_patches/117467/console
For the 400 total runs of each in #7:
AjaxBlockTesthad only a 5% fail rateLayoutBuilderDisableInteractionsTesthad a 17% fail rateLayoutBuilderTesthad an 8% fail rateComment #12
xjmI also think the numbers in #5 are incorrect; there are a total of 43 failures and 2 errors in the results of #3.Sorry, read it the wrong way around.Comment #13
xjmComment #14
xjmMeanwhile: #3268070: Temporarily skip even more failing tests
Comment #15
xjmConverting this to a meta.
Comment #16
xjmComment #17
xjmComment #18
xjmComment #19
lauriiiAdded #3268368: Robustify and restore \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest::testButton to the IS.
Comment #20
phenaproximaAdding #3268678: [random test failure] Restore ContentPreviewToggleTest::testContentPreviewToggle().
Comment #21
phenaproximaAdding #3268680: [random test failure] Restore and fix LayoutBuilderDisableInteractionsTest::testFormsLinksDisabled().
Comment #22
xjm#3268680: [random test failure] Restore and fix LayoutBuilderDisableInteractionsTest::testFormsLinksDisabled() is in, so we can use the mechanism added in that issue and see if it can be applied to other Layout Builder tests (or anything else that tests the off-canvas area).
Comment #23
xjmComment #24
xjmComment #25
phenaproximaComment #26
phenaproximaComment #27
phenaproximaComment #28
phenaproximaAdding #3274053: [random test failure] Restore LayoutBuilderNestedFormUiTest::testAddingFormBlocksToDefaults() to the issue summary.
Comment #30
xjmComment #31
spokjeComment #32
spokjeComment #33
spokjeComment #34
xjmAdding additional skipped tests. Sorry, LB.
Comment #35
bnjmnmComment #36
bnjmnmComment #37
xjmComment #38
xjmComment #40
xjmComment #41
catch#3351494: Skip Drupal\Tests\media\FunctionalJavascript\MediaSourceFileTest need to add here if/when that's committed.
Comment #42
catchSame with #3351500: Skip Drupal\Tests\layout_builder\FunctionalJavascript\ContextualLinksTest
Comment #43
catchComment #44
spokjeComment #45
spokjeComment #46
dwwTried to add #3351596: Skip Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest + #3351597: [random test failure] Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest to the summary, but it was unclear if the list at the top are the follow-up issues to un-skip things, or the original issues where we started skipping (it's some of both).
I added a remaining tasks to start to track which ones still need child issues to get them un-skipped and working.
Maybe it'd be more clear if we had separate lists:
1. Tests we've had to skip
2. Issues to fix + stop skipping them
3. Issues like #2 that are already fixed and the test is active again (the current "Already fixed" list)
? Not sure the best way to track this plan and all its issues... 😅
Thanks/sorry,
-Derek
Comment #47
catchThe three lists idea is good. We don't have issues to unskip all of the tests that we've skipped which is why it's inconsistent at the moment.
Also before opening individual issues, we probably want to try to get #2856047: Avoid random failures in JavascriptTestBase when testing functionality in a dialog done in case that allows us to fix all the dialog tests, and maybe check the related offcanvas logic is used everywhere consistently too. That's a large number of the currently failing/skipped tests at the moment.
Comment #48
xjmComment #49
spokjeComment #50
spokjeComment #51
spokjeComment #52
spokjeComment #53
spokjeComment #54
spokjeComment #55
spokjeComment #56
spokjeComment #57
spokjeComment #58
spokjeComment #59
spokjeComment #60
spokjeComment #61
spokjeComment #62
catchComment #64
spokjeMoved Already fixed issues to the Already fixed section.
(Life can be so easy sometimes...)
Comment #66
catchAll of the issues in the issue summary are fixed now, closing this out!