Disable all animations in Javascript testing

renaming

reroll
moving css for removing transitions from settings_tray_test_css module to this one.
Also including a patch runs \Drupal\Tests\system\FunctionalJavascript\OffCanvasTest 300x with this fix.
Also a patch that run runs \Drupal\Tests\system\FunctionalJavascript\OffCanvasTest 300x without this fix.

Ok only fail in many many tries is mkdir(): File exists
So not at all related.

What do you know? Randomness is Random.

I still think we should do this.

Perhaps we can reuse the option to disable animations that was added in #2316205: Provide a way to disable animations for a11y?

1. +++ b/core/modules/system/tests/modules/css_fix_test/css/css_fix.theme.css
   

   The second line of whitespace at the end of this file needs to be removed.

2. +++ b/core/modules/system/tests/modules/css_fix_test/css/css_fix.theme.css
   @@ -1,9 +1,5 @@
      /* CSS transitions. */
   

   transition: none !important; needs to be added (no browser prefixes needed. Some of my in-progress layout builder tests require this rule to work consistently. Once this rule is added, I can provide a patch with a test that will fail 5-10% of the time without transitions disabled.

3. +++ b/core/modules/system/tests/modules/css_fix_test/css_fix_test.info.yml
   @@ -0,0 +1,6 @@
   +name: 'CSS Test fix'
   

   Unless there are plans to use this module to for other CSS-related test fixes, it would be good to rename it to something that makes its purpose more immediately apparent. Something like css_disable_transitions would make it easy for developers to know at a glance what the module is doing.

re #12

1. fixed
2. fixed
3. fixed change other comments and names to be only about descriptions.

#11
@idebr thanks for letting us know about #2316205: Provide a way to disable animations for a11y

We could just change this patch to set drupalSettings.noAnimate but I don't think we should connect the 2.

The main reason is the solution in that issue could change in the future. Perhaps if they wanted to do something extra that improve a11y concerns relating to animations. In that case those making the change should not have to worry about how that change might affect random testing failures.

Likewise we should able to improve our solution stopping random test failures without worrying about how that would affect a11y concerns. They may be the same now but likely will diverge.

+++ b/core/tests/Drupal/FunctionalJavascriptTests/WebDriverTestBase.php
@@ -14,6 +14,11 @@
+  protected static $modules = ['css_fix_test'];

Forgot to update the module name here

Here is a fix for #14

I am also including 2 versions of the patch from #3002608-51: Remove contextual links not related to layout administration inside layout builder blocks

@bnjmnm has been working on the test for this issue and he says it is good example of a test that fails randomly if animations aren't disabled.

One version has the fix for animations from the issue removed
One version has the fix from this issue added.

Tagging for 'Random test failure' since that's what this is trying to solve.

A few very minor nits. Not worth NW, but if you re-roll anyway, perhaps they could be addressed.

1. +++ b/core/modules/system/tests/modules/css_disable_transitions/css/disable_transitions.theme.css
   --- /dev/null
   +++ b/core/modules/system/tests/modules/css_disable_transitions/css_disable_transitions.info.yml
   
   +++ b/core/modules/system/tests/modules/css_disable_transitions/css_disable_transitions.info.yml
   +++ b/core/modules/system/tests/modules/css_disable_transitions/css_disable_transitions.info.yml
   @@ -0,0 +1,6 @@
   
   @@ -0,0 +1,6 @@
   +name: 'Test disable CSS animations'
   

   Seems like almost everything in core/modules/system/tests/modules has "test" somewhere in the name. I know this is in the "Testing" package, so namespace collisions are unlikely, but perhaps we want "test" somewhere in the module name, too?

2. +++ b/core/modules/system/tests/modules/css_disable_transitions/css_disable_transitions.module
   @@ -0,0 +1,14 @@
   + * Module for attaching CSS during tests.
   

   This description is a bit stale now that the module is named 'css_disable_transitions'.

Otherwise, seems like a very good idea to have this.

Now we just have to make sure folks know when to use it. ;) Perhaps we could add something about this module to the JS testing docs? See #2949715: Automated Tests 8.x topic needs an update for some possible spots we could update. Don't want this to conflict with that, so probably best handled in a followup? For now, also tagging for 'needs documentation updates'. Feel free to disagree and remove that.

Thanks!
-Derek

Once the other feedback is taken care of, I'm inclined to RTBC this with one additional thing: The addition of a designed-to-fail patch. Perhaps one that inflates css transition times enough to cause random failures, even if they're at higher levels than those actually in use. As long as the test demonstrates that CSS transitions are capable of causing intermittent failures, even if they're at higher than normal levels, I think it's sufficient proof that we'd benefit from having them disabled. ( I hear @tedbow might have a patch that does this... )

@dww thanks for review
re #17

1. Changed to css_disable_transitions_test
2. updated to " Helper module for disabling animations in tests"

re #18
Adding a force-fail.patch This adds the same test module but sets all to 2s !important; instead none !important;

I think will cause some test to fail

Wow, I have no idea how I missed this:

 abstract class WebDriverTestBase extends BrowserTestBase {
 
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['css_disable_transitions_test'];
+

Well, that avoids the problem of teaching people to use this! ;) No doc updates required. Love it. Removing that tag.

The force-fail succeeded in failing a lot. ;) A quick skim of the results shows the fails are exactly the target audience for this:

1) Drupal\Tests\ckeditor\FunctionalJavascript\CKEditorIntegrationTest::testFragmentLink
Behat\Mink\Exception\ElementHtmlException: CKEditor-enabled body field is visible.
...
1) Drupal\Tests\layout_builder\FunctionalJavascript\AjaxBlockTest::testAddAjaxBlock
WebDriver\Exception\UnknownError: unknown error: Element is not clickable at point (548, 1081)
...
1) Drupal\Tests\layout_builder\FunctionalJavascript\BlockFormMessagesTest::testValidationMessage
WebDriver\Exception\ElementNotVisible: element not visible
...
1) Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderDisableInteractionsTest::testFormsLinksDisabled
WebDriver\Exception\UnknownError: unknown error: Element is not clickable at point (551, 672)
...

One very minor nit for when you re-upload a clean final patch (to prevent the bot from being confused):

1. + * Remove CSS animation effects that can cause to random test failures.

s/can cause to/can cause/ || s/can cause to/can lead to/

Then this is RTBC.

Thanks!
-Derek

Hrm, although... lots of test classes override this protected $modules member. It's not a method that they can call parent::whatever() on, it's just an array. I don't think our tests usually (ever?) do anything smart about checking for existing values from a parent test class. They just set $modules to what they expect and move on. So, I'm re-adding the tag, since I do think it's worth trying to document this in a few places. Basically, if you're a JS test, and you're defining your own $modules, you probably want to add this one to your list...

#21.1 fixed

RE #22
I have to search for this function to figure out how it works every time but....

\Drupal\Tests\BrowserTestBase::installDrupal() calls \Drupal\Core\Test\FunctionalTestSetupTrait::installModulesFromClassProperty()
This method does some magic🧙‍♂️ to loop through all parent classes and merge all modules from ancestor classes. So you don't have to worry about overwriting the modules property.

This is used by many *TestBase classes such as \Drupal\Tests\taxonomy\Functional\TaxonomyTestBase where any class that extends it doesn't have to actually add taxonomy to $modules.

Basically, if you're a JS test, and you're defining your own $modules, you probably want to add this one to your list...

Removing needs documentation updates tag because the magic in installModulesFromClassProperty() makes it so no other test extending WebDriverTestBase has to worry about it.

Adding credit

I think all that is left is to address the @todo that references this issue in layout_builder_test_css_transitions.info.yml (which only got into HEAD very recently), and this is good to go.

#25 good catch. Done!

RTBC - I think this will prevent many future testing headaches.

@dww I don't think you have to worry about the $modules member variable. The test setup is smart about that, and I forget how, but it goes through and finds all the $modules up the parenting chain of classes (if that makes sense) and includes all of them. Or at least, at one point I was wondering about that and I looked at the code, though it wasn't this particular base class. Anyway I think it is OK.

Yeah, take a look at this method, which is called from WebTestBase::setUp():
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Test%21Fu...

So as long as everyone calls the parent::setUp() method in their setUp(), which they always should, it should be fine.

I hereby retract #22. Thanks @tedbow #23 and @jhodgdon #28. Learn something new every day. ;) Slick.

Agreed on RTBC.

Thanks,
-Derek

I'm hesitant to commit this because the change adds a new testing module that is being enabled by default, but the change looks good from my POV. Leaving for another maintainer to commit.

I have some reservations about making this the default behaviour. Especially making it something that hard to remove. Sometime you might want to test an animation or something I dunno but if there is some logic in the animation or something. I guess the test can uninstall the module.

I would have expected a discussion of the pros and cons of changing the default behaviour somewhere on the issue.

At the very least we should document this behaviour and how to disable it somewhere and we should have a change record to inform developers about the change.

Discovered something else whilst looking at the recent spate of test fails. If we really want to disable all animations we need to disable jQuery animations too. We need the module to stick some js in with

if (window.jQuery) {
  jQuery.fx.off = true;
}

I think to complete this task we need to:

• Somehow allow tests to run with transitions - so the current patches approach of using the $modules property needs adjusting.
• Add the above JS
• Document this behaviour and how to change it in tests

This seems to be happening very frequently for some reason since the weekend, see #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest, should we make this critical or at least major too?

I agree this feels critical at the moment. Here's a patch that does #33. Also it makes tests much faster. For example, LayoutBuilderDisableInteractionsTest is 5 seconds quicker locally because jQuery animations are also disabled.

Drats ... more tests use layout_builder_test_css_transitions now...

Yeah no #36 was wrong too whoops.

Yeah we still have a fail in #37 - https://www.drupal.org/pift-ci-job/1295785 with a not clickable thing.

1. This may be related to the connection to the external site (youtube.com, vimeo.com) or related to the size of the downloaded file.

That's why sometimes random test failure happen more often. Like last weekend. See #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest.

/**
   * Tests that oEmbed media can be added in the Media library's widget.
   */
  public function testWidgetOEmbed() {
    $assert_session = $this->assertSession();
    $page = $this->getSession()->getPage();

    $youtube_title = "Everyday I'm Drupalin' Drupal Rap (Rick Ross - Hustlin)";
    $youtube_url = 'https://www.youtube.com/watch?v=PWjcqE3QKBg';
    $vimeo_title = "Drupal Rap Video - Schipulcon09";
    $vimeo_url = 'https://vimeo.com/7073899';
    ResourceController::setResourceUrl($youtube_url, $this->getFixturesDirectory() . '/video_youtube.json');

2. See also #3045612: Random test failure in MediaStandardProfileTest::testMediaSources.

/**
 * Test the standard profile configuration for media type 'remote_video'.
 */
protected function remoteVideoTest() {
  $assert_session = $this->assertSession();
  $page = $this->getSession()->getPage();
  $source_field_id = 'field_media_oembed_video';

  // Set video fixtures.
  $video_title = 'Drupal Rap Video - Schipulcon09';
  $video_url = 'https://vimeo.com/7073899';
  ResourceController::setResourceUrl($video_url, $this->getFixturesDirectory() . '/video_vimeo.json');

I can't get MediaLibraryTest to pass locally without this patch.

I have two problems...

1. one time instead of clicking on the button it's clicking on the iframe underneath resulting in my hearing the drupalling song
2. if I get past that then there still is a point where a button the test is trying to click on is not in the view port resulting in a fail

Patch attached fixes everythign for me.

So one thing I've discovered is the default screen size on the headless chrome running in DrupalCI is 800x600 and this can result in very different outcomes when running locally and having a different resolution. I'm not sure that 800x600 is a good resolution for us because no-one uses that and when you're debugging tests locally by watching the window this tiny size makes it awkward.

However, as we can see from #41 stopping all the animations and reducing oembed iframes to 0 width and height still has not resolved all the js testing issues :(

One thing that might help is to move the viewport in waitForElementVisible() - I think that we use that to assert that something has appeared somewhere on the page and not really about the viewport. If you want to do viewport assertions I think we should be using assertVisibleInViewport().

I've been following this issue -- I had a bunch of trouble getting the User Guide tests to pass (they are using WebDriverTestBase), due to similar problems that you've seen in the Core tests, but I eventually got them to run. I'm pretty sure you've thought of all of these fixes/hacks, but just in case (or in case anyone stumbles on this issue looking for help), here's a list of things I did to get around the problems in the User Guide tests:

a) Turn off all CSS transitions, transforms, and animations [similar to this patch, but perhaps includes more?]

* {
  /* CSS transitions. */
  -o-transition-property: none !important;
  -moz-transition-property: none !important;
  -ms-transition-property: none !important;
  -webkit-transition-property: none !important;
  transition-property: none !important;
  transition: none !important;
  /* CSS transforms. */
  -o-transform: none !important;
  -moz-transform: none !important;
  -ms-transform: none !important;
  -webkit-transform: none !important;
  transform: none !important;
  /* CSS animations. */
  -webkit-animation: none !important;
  -moz-animation: none !important;
  -o-animation: none !important;
  -ms-animation: none !important;
  animation: none !important;
}

b) Scroll the window up to the top before doing drupalPostForm, visibility tests, or assertText* and friends. Code to do that:

  protected function scrollWindowUp() {
    $this->getSession()->getDriver()->executeScript('window.scroll(0,0);');
  }

  protected function drupalPostForm($path, $edit, $submit, array $options = [], $form_html_id = NULL) {
    $this->scrollWindowUp();
    parent::drupalPostForm($path, $edit, $submit, $options, $form_html_id);
  }

c) A lot of "wait for something to happen before proceeding" code. I wrote this helper function:

  /**
   * Waits for a UI element to be present and ready to interact.
   *
   * The desired interaction is also performed. If it doesn't work out, the
   * method generates a test assertion fail.
   *
   * @param string $type
   *   Type of selector: 'css' or 'xpath'.
   * @param string $selector
   *   Selector for element to wait for.
   * @param string $interaction
   *   The interaction to check for and perform. One of:
   *   - 'click' (default)
   *   - 'focus'
   *   - 'none': Just wait for the element to be visible.
   */
  protected function waitForInteraction($type, $selector, $interaction = 'click') {
    $page = $this->getSession()->getPage();
    $reason = '';
    $result = $page->waitFor(10,
      function() use ($type, $selector, $page, $interaction, &$reason) {
        $item = $page->find($type, $selector);
        if (!$item) {
          $reason = "$type $selector not found on page";
          return FALSE;
        }
        try {
          switch ($interaction) {
            case 'click':
              $item->click();
              return TRUE;
            case 'focus':
              $item->focus();
              return TRUE;
            case 'none':
              return TRUE;
          }
        }
        catch (UnknownError $exception) {
          if (strstr($exception->getMessage(), 'not clickable') === FALSE &&
            strstr($exception->getMessage(), 'not interactable') === FALSE) {
            // Rethrow any unexpected exception.
            throw $exception;
          }
          $reason = "$type $selector found but not ready\n" .
            $exception->getmessage();
          return FALSE;
        }
      });

    if (!$result) {
      $this->fail($reason);
    }
  }

d) A lot of calls to

    $this->assertSession()->assertWaitOnAjaxRequest();

@jhodgdon I'm interested in b) can you point me to a test that was fixed by this? What's interesting is that the code in the webdriver does

    private function clickOnElement(Element $element)
    {
        try {
            // Move the mouse to the element as Selenium does not allow clicking on an element which is outside the viewport
            $this->wdSession->moveto(array('element' => $element->getID()));
        } catch (UnknownCommand $e) {
            // If the Webdriver implementation does not support moveto (which is not part of the W3C WebDriver spec), proceed to the click
        }

        $element->click();
    }

So it'd be really interesting to know why the scrolling helped.

Sure!

The User Guide test base class (which is extended by various subclasses, one per language) is at:
https://git.drupalcode.org/project/user_guide_tests/blob/8.x-7.x/tests/s...
[sorry, it's a huge long class!]. And it has its own base class that defines the scrollWindowUp function (and a few other helpers not relevant to this issue):
https://git.drupalcode.org/project/user_guide_tests/blob/8.x-7.x/tests/s...

Our User Guide tests often follow the pattern of "Test that a bunch of strings we use in the User Guide appear on some admin page, and then fill out and submit the form to test that the steps in the User Guide work". What I was finding was that as I tested the strings on a page, the Chromedriver would scroll the window down to each one as the assertText ran, and then sometimes the test would fail on the next string because it was above the one I had just tested. Scrolling back up to the top fixed that problem.

Similarly, I found when I was submitting forms after doing a bunch of assertText statements, the form fields that were being filled in were not found, and scrolling back up to the top of the page before calling drupalPostForm fixed that problem too. After putting in several calls to my scrollWindowUp() helper function before calling drupalPostForm, I eventually refactored by overriding the function so that I always scrolled up before calling drupalPostForm.

Here's a typical section, so you don't have to read the entire test class:

    // Add content type for Recipe. No screen shots.
    $this->drupalGet('admin/structure/types/add');
    // Open the publishing options section and uncheck "Promoted..".
    $this->clickLink($this->callT('Publishing options'));
    $this->waitForInteraction('css', '#edit-options-promote');
    // Open the display settings section and uncheck 'Display author..'.
    $this->scrollWindowUp();
    $this->clickLink($this->callT('Display settings'));
    $this->waitForInteraction('css', '#edit-display-submitted');
    // Open the menu settings section and uncheck Main navigation menu.
    $this->scrollWindowUp();
    $this->clickLink($this->callT('Menu settings'));
    $this->waitForInteraction('css', '#edit-menu-options-main');
    $this->openMachineNameEdit();
    // Open submission form area and submit the form.
    $this->clickLink($this->callT('Submission form settings'));
    $this->waitForInteraction('css', '#edit-title-label', 'focus');
    $this->waitForInteraction('css', '#edit-save-continue', 'focus');
    $this->drupalPostForm(NULL, [
        'name' => $this->demoInput['recipe_type_name'],
        'type' => $recipe,
        'description' => $this->demoInput['recipe_type_description'],
        'title_label' => $this->demoInput['recipe_type_title_label'],
      ], $this->callT('Save and manage fields'));

There are a few helper functions in there... callT() does some magic to translate things to the language being run in the test, and openMachineNameEdit() is a helper function to open up the "edit machine name" field on a form:

  protected function openMachineNameEdit($name_selector = '#edit-name') {
    $this->getSession()->getDriver()->executeScript("window.scroll(0,0); jQuery('" . $name_selector . "').val('foo'); jQuery('.field-suffix').show(); jQuery('" . $name_selector . "-machine-name-suffix').show();");
    $this->waitForInteraction('css', $name_selector . '-machine-name-suffix button');
    $this->assertSession()->assertWaitOnAjaxRequest();
  }

You can see in there is another window scroll. Really the tests are quite sensitive to this in my experience!

Oh, I should have added (e) to my list in comment #45: When submitting a form with drupalPostForm, or asserting text is there, you have to make sure that the form field you want to fill in is visible. So for instance in a content editing form (and in the example above), if you are filling in something in a vertical tab, that particular vertical tab area needs to be open. And if you want to set the machine name, you need to make sure that part of the form is open. This is not a problem for non-JS tests (using BrowserTestBase for example).

A few unsuccessful tests including #41:

Testing Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderDisableInteractionsTest
F                                                                   1 / 1 (100%)

Time: 28.34 seconds, Memory: 4.00MB

There was 1 failure:

1) Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderDisableInteractionsTest::testFormsLinksDisabled
Element '#drupal-off-canvas' was not on the page after wait.
Failed asserting that false is true.

/**
 * Waits for an element to be removed from the page.
 *
 * @param string $selector
 *   CSS selector.
 * @param int $timeout
 *   (optional) Timeout in milliseconds, defaults to 10000.
 * @param string $message
 *   (optional) Custom message to display with the assertion.
 *
 * @todo: Remove after https://www.drupal.org/project/drupal/issues/2892440
 */
public function assertNoElementAfterWait($selector, $timeout = 10000, $message = '') {
  $page = $this->getSession()->getPage();
  if ($message === '') {
    $message = "Element '$selector' was not on the page after wait.";
  }
  $this->assertTrue($page->waitFor($timeout / 1000, function () use ($page, $selector) {
    return empty($page->find('css', $selector));
  }), $message);
}

There is a similar issue for this #2892440: Provide helper test method to wait for an element to be removed from the page.

Thanks @jhodgdon - interesting but unfortunately not what we're seeing here.

So the MediaLibraryTest testWidgetUpload fails locally for me. To fix it I need to click on a summary to close something that makes the dialog box extend of the bottom of the page and makes the thing not clickable - this is failing at exactly the same point as head!

I've limited the tests to just javascript so we can work out what's going easier and run loads of test runs easily.

Forgot the click :)

Getting there.

Let's see if we can take a screenshot.

Looks like we're down to one JS related fail in \Drupal\Tests\layout_builder\FunctionalJavascript\BlockFormMessagesTest::testValidationMessage() and there's no doubt the current patch is a massive improvement.

Let's get a screenshot after waiting...

Here's the screenshot when it fails https://dispatcher.drupalci.org/job/drupal_patches/98525/artifact/jenkin... and here's when it passes https://dispatcher.drupalci.org/job/drupal_patches/98516/artifact/jenkin... ... so not enough info :(

Updating issue summary will the current 2 tests that have failed... can't get BlockFormMessagesTest to fail with extra instrumentation though... :(

Add some instrumentation for the other fail.

I'm not sure that the \Drupal\Tests\layout_builder\FunctionalJavascript\LayoutBuilderDisableInteractionsTest::assertNoElementAfterWait() is good - I've updated it to match other implementations.

Also revert the screen size change in the other test to see if that is what "fixed" it.

More test runs..

Yay so the random fail in BlockFormMessagesTest::testValidationMessage() is back - very interestingly the powered by block is missing on https://dispatcher.drupalci.org/job/drupal_patches/98644/artifact/jenkin...

Looking at the html again - we have a Placeholder for the "New title" block but we're expecting the block to have been replaced real block and be identified by #layout-builder .block-system-powered-by-block

So now we have just BlockFormMessagesTest::testValidationMessage() going to put the window height change back to see if it fixes it - not sure why it would tbh as #64 shows this is about the placeholders not being replaced.

Added issues to @todos

• #3055982: Remove resizing window in BlockFormMessagesTest::testValidationMessage
• #3055983: Occasional locks on SQLite
• Made #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest a follow-up

I think we should commit this patch to get some stability in HEAD and over time revisit the @todos. This patch result in far far less random fails than we have in HEAD. It's not perfect but atm any move towards less fails seems a good idea.

Created the change record https://www.drupal.org/node/3055990 and noticed a missing @var on review.

See the number of fails of the test only patch in #67 compared to #66 for why doing this is the right thing even though it is not perfect.

+++ b/core/modules/system/tests/modules/css_disable_transitions_test/css/disable_transitions.theme.css
@@ -25,3 +26,11 @@
+/**
+ * Prevent youtube and third party content in iFrames from affecting tests.
+ */
+iframe.media-oembed-content {
+  width: 0 !important;
+  height: 0 !important;
+}

It some ways this is the most contentious part of this change. For me it was only necessary in order to make \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest::testWidgetUpload work locally. DrupalCI didn't fail in the same place because it uses a different screen size. I think we need to add an issue to set the window size so all JS tests are run in window the same size by default. I've opened #3056008: Set default window size in \Drupal\FunctionalJavascriptTests\WebDriverTestBase to address this.

Changes make sense and look good, we have follow ups to address the new @todo's and other found issues.

+++ b/core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFormMessagesTest.php
@@ -38,6 +38,9 @@ protected function setUp() {
+    // @todo Work why this fixes random fails in this test.

Nit 'work out'.

+++ b/core/modules/system/tests/modules/css_disable_transitions_test/css/disable_transitions.theme.css
@@ -25,3 +26,11 @@
+ * Prevent youtube and third party content in iFrames from affecting tests.

Do we need an issue to shift oembed testing to using a local file or similar?

Addresses #72.1 - the second part of that is worth discussing in follow-ups - I've already got some of that in #3056008: Set default window size in \Drupal\FunctionalJavascriptTests\WebDriverTestBase

Oh yeah. RE #70, the User Guide tests do set the window size to a specific value. But we had a different reason -- our tests are dual purpose: (a) test the text/steps in the User Guide and (b) make screenshots for the User Guide. Because of (b) we needed the window to be always a standard size (1200x800 in our case).

    $this->getSession()->resizeWindow(1200, 800);

Committed f4b3ab3 and pushed to 8.8.x. Thanks!

Leaving RTBC against 8.7.x for cherry-pick after the freeze.

👏

Just opened #3056536: LayoutBuilderDisableInteractionsTest randomly fails as a result of last nights 8.8.x core branch testing - which is much much better but we still have that one.

Cherry-picked to 8.7.x, thanks!

We have JavaScript test fails in Thunder distribution with 8.7.3 release. Problem is following.

For Thunder distribution we have custom admin theme and one of differences from default Seven theme is that sidebar (tray) on article edit form can be collapsed/expanded. We are using following CSS transform: translate3d(-100%,0,0); and with new CSS styles introduced with css_disable_transitions_test it's overwritten and whole sidebar stays invisible.

We will probably use $disableCssAnimations = FALSE; in our tests because we want to run our JavaScript tests in the same environment as it would be for real user.

I would like to point out that, these changes could lead to contrib module tests failures too. And I'm not sure if such change should be introduced with bug fix release. :(