Test the upgrade path

I am installing Drupal 6 with http://www.os-cms.net/content/article/view/1 this script and then cp sites/default/default.settings.php sites/default/settings.php ; chmod a+w sites/default/settings.php ;mysql -e 'drop database drupal6'; php install_drupal.php (i dont need to create the db, it's on tmpfs hence the drop cant delete the db ony the tables. handy.) and then see attached script for dumping.

Critical because we are fixing a bug too. Added the latest dumper here with the users table fix.

Well, of course, if you remove the array keys :) so who wants to RTBC this itsy-bitsy patchy?

+1 to RTBC.

Can't find anything wrong with it myself. Looks pretty awesome.

#8: 838438.patch queued for re-testing.

Wow!! This is GREAT! :D

The one major thing missing for me is a README.txt or... something. That explains how to create this for each version, and also how to keep this updated through the ages, not only for minor schema adjustments in the version previous, but also for when we start shipping a Drupal 7 schema for Drupal 8.

For example, I have no idea how you made that drupal6_bare.sql.php file. I assume it's from the result of those dumper scripts, but then we probably ought to store the dumper script in along with the test rather than having to find issue #838438 to re-do this in Drupal 8.

Is it a full schema dump of a stock installation of Drupal 6? Looks like not, since I see "nk"'s user record in there (not sure how you got uid 2 ;P). So if not, why are some things different than others, and can we document when that's the case..?

Also, which version of Drupal 6 was this against? We don't have that documented anywhere in the test or at the top of the sql.php file. Incidentally, can we rename that drupal_6.schema.php? There's no SQL in there from what I can see.

Then, each release of Drupal 7, do I need to re-run this dump file on whatever the current version of Drupal 6 is, or how does this get updated as Drupal 6's schema changes..?

Finally, could we get some docblock on +class BasicUpgradePath extends UpgradePathTestCase { to explain what "Basic" upgrade path means..?

Sorry to mark it needs work for essentially documentation, but I want to strike while you both are in the same place. :P Once you go home, the chances of this documentation materializing are slim to none, and we absolutely need it in order for this test to be updated through the life of Drupal 7, 8, 9...

quick note: I got uid 2 because the dumper increases uid by 1 and then issues an UPDATE users SET uid = uid - 1 to avoid writing a 0 into a serial. MySQL messes that up.

The dumper script is there, there is a description of how the D6 dump was created, it's a stock install now. I think we are good to go.

I added moooore comments to the dumper script to make webchick feel better :)

Edit: the dumper script is at the end of the patch.

Marked #377856: Provide D6 to D7 update test as duplicate. Appears to be doing the same thing.

This is amazing stuff. Thank you, DamZ and chx!

I spent about an hour reviewing this patch but lost my review thanks to a Dreditor/Drupal.org JS conflict. Sigh.

We went through most of it on IRC. The latest patch here reflects the following changes:

1. The includes/variable.inc file was missing. Now added.

2. Also renamed it to 'utility.inc' since it's not actually for variable-related functions. This file allows us to add seldomly-used, yet handy functions that we don't want loaded on every page request, vs. bootstrap.inc or common.inc which are.

3. The comment above the database.php file said it was generated from a stock Drupal install, but that wasn't the case; only dblog and update were enabled. This was fixed. This will help future maintenance of this test.

Waiting for testbot to come back green and then this looks good to go.

Also, some follow-up that would be nice:

+  // Don't output values for those tables.
+  if (substr($table, 0, 5) == 'cache' || $table == 'sessions' || $table == 'watchdog') {

It'd be convenient if the 'exempt' tables were moved closer to the top of the script in a variable that could be easily changed. This changes between major (and sometimes minor) versions of Drupal.

+  // It was previously possible for a value of "0" to be stored in database
+  // tables to indicate that a particular piece of text should be filtered
+  // using the default text format.
+  $default_format = variable_get('filter_default_format', 1);
+  db_update('block_custom')
+    ->fields(array('format' => $default_format))
+    ->condition('format', 0)
+    ->execute();

This code is copy/pasted multiple times in the patch, and presumably also exists in node.install somewhere. We should probably pull it into a helper function, since that would reduce code duplication in core, and also offer the ability for contrib to use it as well.

Ok, testbot shows we're good. Committed to HEAD, with minor comment massaging. Great job!

Wrong patch damien :)

yay for unbroken HEAD.

Committed to HEAD. Thanks!

Where did this change from 0 as default format to a specified number happened? I've submitted a head2head issue for this so we make sure to investigate. This is not at all mentioned/discussed in this issue unfortunately. See http://drupal.org/node/845434

For anyone that might need it, here's a script to dump files from drupal 7. I need it in order to write a test for a hook_update_N(). The change in the script is minor:
- Change include to include_once dirname(__FILE__) . '/includes/utility.inc';
- Remove db_result_array()

remove .txt from file name.

Nope, it's actually not working. This one is a bit better, but I still get DatabaseSchemaObjectExistsException: Cannot add field <em class="placeholder">sessions</em>.<em class="placeholder">ssid</em>: field already exists error when I try to import it via simpletest.

So:
1) Here's the adapted dump script that works.
2) Also attached the simpletest class that I use for OG, which extends UpgradePathTestCase. UpgradePathTestCase has some D6 related hacks, so we can simply remove them.