Problem/Motivation

Some examples of random test failure by MediaStandardProfileTest::testMediaSources

Test the standard profile configuration for media type 'remote_video'.
https://www.drupal.org/pift-ci-job/1250132
https://www.drupal.org/pift-ci-job/1210218

There was 1 failure:

1) Drupal\Tests\media\FunctionalJavascript\MediaStandardProfileTest::testMediaSources
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'Drupal Rap Video - Schipulcon09'
+'example_2.jpeg'

/var/www/html/core/modules/media/tests/src/FunctionalJavascript/MediaStandardProfileTest.php:400
/var/www/html/core/modules/media/tests/src/FunctionalJavascript/MediaStandardProfileTest.php:80

Test the standard profile configuration for media type 'image'.
https://www.drupal.org/pift-ci-job/1209481

There was 1 failure:

1) Drupal\Tests\media\FunctionalJavascript\MediaStandardProfileTest::testMediaSources
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'example_2.jpeg'
+'example_1.jpeg'

/var/www/html/core/modules/media/tests/src/FunctionalJavascript/MediaStandardProfileTest.php:249
/var/www/html/core/modules/media/tests/src/FunctionalJavascript/MediaStandardProfileTest.php:79

Proposed resolution

Add waitForLink($locator, $timeout = 10000) after pressButton('Save').

     $page->pressButton('Save');
+    $result = $assert_session->waitForLink('Add media');
+    $this->assertNotEmpty($result);
+    $assert_session->addressEquals('admin/content/media');

Remaining tasks

None

CommentFileSizeAuthor
#75 3045612-2-75.patch2.68 KBalexpott
#73 3045612-2-does-it-change2.patch2.83 KBalexpott
#73 3045612-2-does-it-change.patch2.78 KBalexpott
#70 3045612-2-restart-networking.patch2.71 KBalexpott
#69 3045612-2-restart-networking.patch2.7 KBalexpott
#66 3045612-65.patch1.27 KBalexpott
#64 3045612-2-test-MediaStandardProfileTest-fix.patch2.66 KBalexpott
#62 3045612-3-remove-search.patch2.04 KBalexpott
#62 3045612-3-test-only.patch1.81 KBalexpott
#61 3045612-edit-resolv.patch10.19 KBalexpott
#60 3045612-all-js-with-hosts-fix.patch10.33 KBalexpott
#59 3045612-onwards.patch10.44 KBalexpott
#57 3045612-onwards.patch10.45 KBalexpott
#55 3045612-onwards.patch10.39 KBalexpott
#53 3045612-onwards.patch10.38 KBalexpott
#51 3045612-onwards.patch10.36 KBalexpott
#50 3045612-onwards.patch10.39 KBalexpott
#49 3045612-locate-ethtool2.patch10.22 KBalexpott
#43 3045612-locate-ethtool.patch10.22 KBalexpott
#41 3045612-concurrency-5.patch10.19 KBalexpott
#41 3045612-aws-instrumentation.patch10.17 KBalexpott
#39 3045612-39-increase-dns-timeout.patch9.96 KBalexpott
#34 3045612-34-stop-hiding-the-exception.patch9.38 KBalexpott
#30 3045612-29-stop-hiding-the-exception.patch9.37 KBalexpott
#28 3045612-28-sleep-100-times.patch8.37 KBalexpott
#28 3045612-28-no-sleep-100-times.patch8.37 KBalexpott
#27 3045612-27.patch8.37 KBalexpott
#21 3045612-21.patch7.89 KBalexpott
#5 3045612-test-only.patch716 bytesKrzysztof Domański
#4 interdiff-2-4.txt2.23 KBKrzysztof Domański
#17 3045612-17.test-only.patch2.03 KBalexpott
#19 3045612-19.patch3.15 KBalexpott
#4 3045612-4.patch3.17 KBKrzysztof Domański
#5 3045612-5.patch4.12 KBKrzysztof Domański
#17 3045612-17.patch2.77 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Krzysztof Domański created an issue. See original summary.

Krzysztof Domański’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
3.18 KB

-- edited --

Status: Needs review » Needs work

The last submitted patch, 2: 3045612-2.patch, failed testing. View results

Krzysztof Domański’s picture

Status: Needs work » Needs review
FileSize
2.23 KB
3.17 KB
Krzysztof Domański’s picture

1. Ignore previous patches. Another cause of the random test failure.

2. Looks like problem with pressButton('Save'). We should wait to make sure the media will be added.

// Set video fixtures.
$video_title = 'Drupal Rap Video - Schipulcon09';

(...)

// Create a media item.
$page->fillField("{$source_field_id}[0][value]", $video_url);
$page->pressButton('Save');
$remote_video_media_id = $this->container
  ->get('entity_type.manager')
  ->getStorage('media')
  ->getQuery()
  ->sort('mid', 'DESC')
  ->execute();
$remote_video_media_id = reset($remote_video_media_id);

(...)

// Check if the default media name is generated as expected.
$media = \Drupal::entityTypeManager()->getStorage('media')->loadUnchanged($remote_video_media_id);
$this->assertSame($video_title, $media->label());
--- Expected
+++ Actual
@@ @@
-'Drupal Rap Video - Schipulcon09'
+'example_2.jpeg'

/var/www/html/core/modules/media/tests/src/FunctionalJavascript/MediaStandardProfileTest.php:400
/var/www/html/core/modules/media/tests/src/FunctionalJavascript/MediaStandardProfileTest.php:80

3. See also:
#2936432: Review use of pressButton() in functional javascript tests in the Media module
#2934997: Intermittent failure in MediaUiJavascriptTest

The last submitted patch, 5: 3045612-test-only.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Krzysztof Domański’s picture

Issue summary: View changes
Krzysztof Domański’s picture

Priority: Critical » Normal
Related issues: +#3059022: If Vimeo is down our tests break

#3059022: If Vimeo is down our tests break should solve the problem when Vimeo is down.

However, this may not be enough. See https://www.drupal.org/pift-ci-job/1209481. This is "Test the standard profile configuration for media type 'image'". The test failure is not related to vimeo.

There was 1 failure:

1) Drupal\Tests\media\FunctionalJavascript\MediaStandardProfileTest::testMediaSources
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'example_2.jpeg'
+'example_1.jpeg'

/var/www/html/core/modules/media/tests/src/FunctionalJavascript/MediaStandardProfileTest.php:249
/var/www/html/core/modules/media/tests/src/FunctionalJavascript/MediaStandardProfileTest.php:79
/**
 * Test the standard profile configuration for media type 'image'.
 */
protected function imageTest() {
  $assert_session = $this->assertSession();
  $page = $this->getSession()->getPage();
  $source_field_id = 'field_media_image';

  (...)

  // Assert the media name is updated through the field mapping when changing
  // the source field.
  $this->drupalGet('media/' . $image_media_id . '/edit');
  $page->pressButton('Remove');
  $result = $assert_session->waitForField("files[{$source_field_id}_0]");
  $this->assertNotEmpty($result);
  $image_media_name_updated = 'example_2.jpeg';
  $page->attachFileToField("files[{$source_field_id}_0]", $this->root . '/core/modules/media/tests/fixtures/' . $image_media_name_updated);
  $result = $assert_session->waitForButton('Remove');
  $this->assertNotEmpty($result);
  $page->fillField("{$source_field_id}[0][alt]", 'Image Alt Text 2');
  $page->pressButton('Save');

  $this->drupalGet('/node/' . $node->id());

  // Check if the default media name is updated as expected.
  $media = \Drupal::entityTypeManager()->getStorage('media')->loadUnchanged($image_media_id);
  $this->assertSame($image_media_name_updated, $media->label()); // line 249

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

alexpott’s picture

Category: Task » Bug report
Priority: Normal » Critical

This is still happening today.

alexpott’s picture

I think the problem here is that we're going to the next page prior to checking that the save has finished.

Status: Needs review » Needs work

The last submitted patch, 17: 3045612-17.patch, failed testing. View results

alexpott’s picture

Status: Needs work » Needs review
FileSize
3.15 KB

Missed a place where we are pressing a button... and then not checking that it is done...

Status: Needs review » Needs work

The last submitted patch, 19: 3045612-19.patch, failed testing. View results

alexpott’s picture

Status: Needs work » Needs review
FileSize
7.89 KB

Status: Needs review » Needs work

The last submitted patch, 21: 3045612-21.patch, failed testing. View results

alexpott’s picture

So here's an image of what is happening when the issue fails...
https://dispatcher.drupalci.org/job/drupal_patches/152779/artifact/jenki...

We get an error "The provided URL does not represent a valid oEmbed resource".

andypost’s picture

Probably that's a cause of error when fetching resource, maybe it's caused by rate limit or quota limits?

https://developers.google.com/youtube/v3/determine_quota_cost

core/modules/media/src/Plugin/Validation/Constraint/OEmbedResourceConstraintValidator.php-115-    try {
core/modules/media/src/Plugin/Validation/Constraint/OEmbedResourceConstraintValidator.php-116-      $resource_url = $this->urlResolver->getResourceUrl($url);
core/modules/media/src/Plugin/Validation/Constraint/OEmbedResourceConstraintValidator.php-117-      $this->resourceFetcher->fetchResource($resource_url);
core/modules/media/src/Plugin/Validation/Constraint/OEmbedResourceConstraintValidator.php-118-    }
core/modules/media/src/Plugin/Validation/Constraint/OEmbedResourceConstraintValidator.php-119-    catch (ResourceException $e) {
core/modules/media/src/Plugin/Validation/Constraint/OEmbedResourceConstraintValidator.php:120:      $this->handleException($e, $constraint->invalidResourceMessage);
core/modules/media/src/Plugin/Validation/Constraint/OEmbedResourceConstraintValidator.php-121-    }
andypost’s picture

I can't find reference to rate limits but there's https://developers.google.com/youtube/v3/getting-started#quota

Projects that enable the YouTube Data API have a default quota allocation of 10,000 units per day, an amount sufficient for the overwhelming majority of our API users.

alexpott’s picture

@andypost we mock all of the requests... I can run this test successfully with no internet. Also that wouldn't explain how this is random - it would start failing at some point during the day and then stop failing.

alexpott’s picture

alexpott’s picture

The last submitted patch, 27: 3045612-27.patch, failed testing. View results

alexpott’s picture

The last submitted patch, 28: 3045612-28-no-sleep-100-times.patch, failed testing. View results

The last submitted patch, 28: 3045612-28-sleep-100-times.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 30: 3045612-29-stop-hiding-the-exception.patch, failed testing. View results

alexpott’s picture

Status: Needs work » Needs review
FileSize
9.38 KB

So the request to media_test_oembed/resource is failing for some reason!

alexpott’s picture

Here's the exception we're getting from #30 - https://dispatcher.drupalci.org/job/drupal_patches/152811/artifact/jenki...

Hopefully #34 will give us more info.

Status: Needs review » Needs work

The last submitted patch, 34: 3045612-34-stop-hiding-the-exception.patch, failed testing. View results

alexpott’s picture

So https://dispatcher.drupalci.org/job/drupal_patches/152812/artifact/jenki... is very interesting...

A local DNS resolution is taking move than 5000ms! I think something is wrong with the container and DNS.

alexpott’s picture

Here's an even easier to read version of the exception... https://dispatcher.drupalci.org/job/drupal_patches/152812/artifact/jenki...

Exception: Could not retrieve the oEmbed resource. cURL error 28: Resolving timed out after 5000 milliseconds (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for http://php-apache-jenkins-drupal-patches-152812/subdirectory/media_test_oembed/resource?url=https://www.youtube.com/watch?v=PWjcqE3QKBg in Drupal\media\Plugin\Validation\Constraint\OEmbedResourceConstraintValidator->handleException() (line 148 of core/modules/media/src/Plugin/Validation/Constraint/OEmbedResourceConstraintValidator.php).
alexpott’s picture

Status: Needs work » Needs review
FileSize
9.96 KB

Let's try increasing the timeout.

Status: Needs review » Needs work

The last submitted patch, 39: 3045612-39-increase-dns-timeout.patch, failed testing. View results

alexpott’s picture

The last submitted patch, 41: 3045612-aws-instrumentation.patch, failed testing. View results

alexpott’s picture

  • alexpott committed 2b24cf9 on 10.1.x
    Issue #3045612 by alexpott, Krzysztof Domański: Random test failure in...

  • alexpott committed 9218e47 on 10.0.x
    Issue #3045612 by alexpott, Krzysztof Domański: Random test failure in...

  • alexpott committed 260dfe8 on 9.5.x
    Issue #3045612 by alexpott, Krzysztof Domański: Random test failure in...

  • alexpott committed 204055b on 9.4.x
    Issue #3045612 by alexpott, Krzysztof Domański: Random test failure in...
alexpott’s picture

I've made a commit to 10.1.x through to 9.4.x to reduce JS test concurrency based on #41.

alexpott’s picture

alexpott’s picture

alexpott’s picture

Status: Needs review » Needs work

The last submitted patch, 51: 3045612-onwards.patch, failed testing. View results

alexpott’s picture

Status: Needs work » Needs review
FileSize
10.38 KB

Status: Needs review » Needs work

The last submitted patch, 53: 3045612-onwards.patch, failed testing. View results

alexpott’s picture

Status: Needs work » Needs review
FileSize
10.39 KB

Status: Needs review » Needs work

The last submitted patch, 55: 3045612-onwards.patch, failed testing. View results

alexpott’s picture

alexpott’s picture

So #57 is a good fix for requests back to the PHP container - but we still have the chromedriver container to contend with. The other random fails are to do with slowness there and given the issue identified here it might well be DNS too.

alexpott’s picture

Let's see if resolv.conf gives us some clues.

alexpott’s picture

Let's see if we can fix resolv.conf

The whole problem is here:

search us-west-2.compute.internal
nameserver 127.0.0.11
options ndots:0

The line search us-west-2.compute.internal causes DNS lookups to append .us-west-2.compute.internal to every lookup we do... so before we lookup php-apache-jenkins-drupal-patches-152812 we lookup php-apache-jenkins-drupal-patches-152812.search us-west-2.compute.internal - the docker nameserver will fall back to amazon's at this point and then eventually we will get rate limited.

We need to prevent this from happening on all the containers but let's start with the PHP container.

The last submitted patch, 61: 3045612-edit-resolv.patch, failed testing. View results

alexpott’s picture

The last submitted patch, 62: 3045612-3-remove-search.patch, failed testing. View results

alexpott’s picture

I've created #3316016: DNS lookups on containers can be throttled to fix this on the containers properly but here's a fix for the PHP container... I don't think it is early enough to help the chromedriver container so I still expect this to cause the occasional random fail during heavy JS testing.

And committing the patch here will certainly do no harm. But we need to ensure that this change doesn't break any of the other test types. I'm pretty sure it won't but you never know.

The last submitted patch, 64: 3045612-2-test-MediaStandardProfileTest-fix.patch, failed testing. View results

The last submitted patch, 62: 3045612-3-test-only.patch, failed testing. View results

alexpott’s picture

Oh maybe we have to restart networking... interesting.

alexpott’s picture

The last submitted patch, 69: 3045612-2-restart-networking.patch, failed testing. View results

andypost’s picture

btw in new releases of Debian this file is generated and symlinked so edits are not saved

lrwxrwxrwx 1 root root 39 сен 30 17:42 resolv.conf -> ../run/systemd/resolve/stub-resolv.conf

alexpott’s picture

@andypost yeah I was wondering about that... obviously I am managing to change it... but does something come along and revert it.

Status: Needs review » Needs work

The last submitted patch, 73: 3045612-2-does-it-change2.patch, failed testing. View results

  • alexpott committed c0e89d1 on 10.1.x
    Revert "Issue #3045612 by alexpott, Krzysztof Domański: Random test...

  • alexpott committed 2d36c6f on 10.0.x
    Revert "Issue #3045612 by alexpott, Krzysztof Domański: Random test...

  • alexpott committed ffd3b8f on 9.5.x
    Revert "Issue #3045612 by alexpott, Krzysztof Domański: Random test...

  • alexpott committed 46c00ce on 9.4.x
    Revert "Issue #3045612 by alexpott, Krzysztof Domański: Random test...
Wim Leers’s picture

alexpott’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

quietone’s picture