We need RC1 database dumps so that we can specifically target the upgrade path added in the RC phase. For example, system_post_update_recalculate_configuration_entity_dependencies() updates all configuration entities making it hard to test fixing dependencies.

This is rc eligible because it is test only changes.

CommentFileSizeAuthor
#11 2592665-11.patch746.66 KBjhedstrom
#11 interdiff.txt1.9 KBjhedstrom
#5 2592665-5.patch745.84 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review
Issue tags: +Needs tests

Created dumps by:

  1. Checking RC1
  2. Uploading the corresponding db dump
  3. Running update.php
  4. Running dump-database-d8-mysql.php

I guess a test per file would not go amiss.

catch’s picture

Do we want a new database with similar contents to the beta12 ones?

Or do we want to upgrade the beta12 database to RC1 then dump it?

If there are bugs in the beta12-rc upgrade path which are undiscovered, those could be two different database dumps...

alexpott’s picture

Status: Needs review » Needs work

I guess with the filled dong the steps I did in #2 makes sense - we have to scripts to create all the content because we don't do it like that. But with the bare we should just install standard at RC1 and dump it. I'll re-do the bare dump.

alexpott’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new745.84 KB

Added tests - as there are no updates yet these tests don't show anything but having the dumps tests and ready waiting for the first update hooks will be helpful for issues like #2479487: ImageStyles can be deleted while having dependent configuration.

Also the bare database dump was generated because installing standard at the point of RC1 and dumping the db.

dawehner’s picture

One thing which would be interesting is whether the dump of the DB after beta 12 + update path is basically the same compared to the DB installed on RC1 directly.

effulgentsia’s picture

Priority: Major » Critical

Even if this isn't technically Critical until there's some other issue that adds an update function, I still think it makes sense to raise this to Critical now, since we'll have such an issue eventually.

I think it also makes a lot of sense for this to happen before tagging RC2, though if it doesn't, we can retitle it to "Create RC2 database dumps" after tagging RC2.

dawehner’s picture

Yeah RC1 is now already nearly history.

IMHO we should still keep the old dumps so we ensure that continues update paths are supported, not only to support THIS continues update path.

catch’s picture

I think we should open a follow-up, possibly meta, to add more database dumps. As we have more releases, we have more possible code paths for updates.

Assuming two weeks in the future and RC3 has been tagged, all of the following are possible:

beta12-rc3

beta12-rc1-rc3

beta12-rc1-rc2-rc3

rc1-rc3

rc1-rc2-rc3

rc2-rc3

Fortunately for us, we no-longer have 7.40->8.0.0-rc3, and hence no 5.2-6.43-7.40-8.0.0-rc3.

There are diminishing returns with trying to test that many upgrade path routes (although really, they don't diminish that much), and for real sites we can tell people to never skip a core release (which reduces the number of permutations above), but it'd be great to get to the point where we can add those permutations more easily.

I think what's here is a good improvement though, and we should get it in as-is.

dawehner’s picture

+  /**
+   * Tests that the database was properly loaded.
+   */
+  public function testDatabaseLoaded() {
+    $hook_updates = [
+      'user' => '8000',
+      'node' => '8003',
+      'system' => '8013',
+    ];
+    foreach ($hook_updates as $module => $schema) {
+      $this->assertEqual(drupal_get_installed_schema_version($module), $schema, SafeMarkup::format('Module @module schema is @schema', ['@module' => $module, '@schema' => $schema]));
+    }
+

Maybe we could also check that we have the hook_post_update_X() executed as well
Let's not use SafeMarkup::format() but new FormattableMarkup()

jhedstrom’s picture

StatusFileSize
new1.9 KB
new746.66 KB

This should address #10.

catch’s picture

Status: Needs review » Reviewed & tested by the community

Bumping this to RTBC.

But we should open a follow-up, before actually committing this, to add scriptable filled database dumps - since otherwise we're stuck with:

1. Only having test coverage for a site starting on beta 12, which is going to be 0.0000001% of actual Drupal 8 sites

2. Having to manually recreate the dump by clicking around if we want anything other than #1.

  • effulgentsia committed b6072fe on 8.0.x
    Issue #2592665 by jhedstrom, alexpott: Create RC1 database dumps
    
effulgentsia’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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