Problem/Motivation

As discovered in #2976035: Entity type CRUD operations must use the last installed entity type and field storage definitions , \Drupal\Tests\media\Functional\Update\MediaUpdateTest does a few things wrongly:

  1. drupal-8.media-enabled.php installs live config definitions from the standard profile. That's a big no-no for upgrade path tests since the "installed site fixture" is a moving target, which makes test results completely unpredictable
  2. MediaUpdateTest::testOEmbedConfig() tries to bypass the problem described above by removing data from a config file before running the updates. That data should not have been there in the first place
  3. MediaUpdateTest::testBundlePermission() changes a role config entity via API calls before running the updates, which is not allowed/supported

Proposed resolution

- replace drupal-8.media-enabled.php with a database fixture which installs only the objects created by the Media module in Drupal 8.4.0
- add additional permissions to the authenticated role in an update fixture file instead of using API calls before running the updates
- don't change the media.settings config file before running the updates, because the database dump from above includes the correct file and data for it

Remaining tasks

Review.

User interface changes

Nope.

API changes

Nope.

Data model changes

Nope.

CommentFileSizeAuthor
#3 interdiff-3.txt249.39 KBamateescu
#3 2989469-3.patch150.99 KBamateescu
#2 2989469.patch196.91 KBamateescu

Comments

amateescu created an issue. See original summary.

amateescu’s picture

Status: Active » Needs review
StatusFileSize
new196.91 KB

And a patch.

amateescu’s picture

Issue summary: View changes
StatusFileSize
new150.99 KB
new249.39 KB

When proposing the new db dump I was under the impression that Content Moderation also had one. Turns out that's not the case, so let's not add one here as well.

samuel.mortenson’s picture

Status: Needs review » Reviewed & tested by the community

This makes sense to me and unblocks an important issue. Marking as RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

As a test only change backported to 8.6.x

Committed and pushed bfb517693e to 8.7.x and 530982ad66 to 8.6.x. Thanks!

  • alexpott committed bfb5176 on 8.7.x
    Issue #2989469 by amateescu: MediaUpdateTest is broken
    

  • alexpott committed 530982a on 8.6.x
    Issue #2989469 by amateescu: MediaUpdateTest is broken
    
    (cherry picked...

Status: Fixed » Closed (fixed)

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