while testing the upgrade path from aggregator categories to entity reference field for #15266: Replace aggregator category system with taxonomy i found out that
aggregator_update_8000 has the wrong name:)
see patch below

Files: 
CommentFileSizeAuthor
#15 drupal-aggregator_update-1958702-13.patch2.69 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 54,088 pass(es). View
#11 drupal-aggregator_update-1958702-11.patch2.69 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 53,861 pass(es), 1 fail(s), and 0 exception(s). View
#7 drupal-aggregator_update-1958702-7-fail.patch1.38 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 53,917 pass(es), 1 fail(s), and 30 exception(s). View
#7 drupal-aggregator_update-1958702-7.patch1.9 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 53,909 pass(es), 1 fail(s), and 18 exception(s). View
#1 drupal-aggregator_update-1958702-1.patch530 bytesParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 53,927 pass(es). View

Comments

ParisLiakos’s picture

Status: Active » Needs review
FileSize
530 bytes
PASSED: [[SimpleTest]]: [MySQL] 53,927 pass(es). View
ParisLiakos’s picture

Title: Aggregato variables to config update is never executed » Aggregator variables to config update is never executed
Berdir’s picture

Sounds like we should add those to the config upgrade path test class?

ParisLiakos’s picture

Issue tags: +Configuration system

trying to find where the test class for variable to config is, but i only find ConfigUpgradeTest which tests fake variables.
do we have any tests on real variables?

Berdir’s picture

Not sure if I understand what you mean with fake variables. Those variables are as real as you make them, the test class just contains what to assert, you need to add them to drupal-7.system.database.php so that they actually exist before the update is run.

ParisLiakos’s picture

ah i see..with fake, i mean variables not used by real drupal sites:)
in that case i dont think we should add them there, if we add aggregator variables we should include all modules variables:/

Edit: i was looking the wrong test...SystemUpgradePathTest is the correct one

ParisLiakos’s picture

FileSize
1.9 KB
FAILED: [[SimpleTest]]: [MySQL] 53,909 pass(es), 1 fail(s), and 18 exception(s). View
1.38 KB
FAILED: [[SimpleTest]]: [MySQL] 53,917 pass(es), 1 fail(s), and 30 exception(s). View

sth like this i guess

Berdir’s picture

While you're at it, can you add the other variables as well?

Status: Needs review » Needs work

The last submitted patch, drupal-aggregator_update-1958702-7.patch, failed testing.

ParisLiakos’s picture

yeah..i am supposed to serialize it first -.-

ParisLiakos’s picture

Status: Needs work » Needs review
FileSize
2.69 KB
FAILED: [[SimpleTest]]: [MySQL] 53,861 pass(es), 1 fail(s), and 0 exception(s). View

lets hope i did them correct:p

Status: Needs review » Needs work

The last submitted patch, drupal-aggregator_update-1958702-11.patch, failed testing.

Berdir’s picture

Status: Needs work » Reviewed & tested by the community

This looks good if it passes.

Berdir’s picture

Status: Reviewed & tested by the community » Needs work

Hah, I guess not then.

ParisLiakos’s picture

Status: Needs work » Needs review
FileSize
2.69 KB
PASSED: [[SimpleTest]]: [MySQL] 54,088 pass(es). View

yeah alright missed a zero:P

Berdir’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/SystemUpgradePathTest.phpundefined
@@ -159,6 +159,16 @@ public function testVariableUpgrade() {
+      'items.teaser_length' => 6000,

+++ b/core/modules/system/tests/upgrade/drupal-7.system.database.phpundefined
@@ -135,6 +135,34 @@
+    'value' => 'i:600;',

6000 vs. 600.

Berdir’s picture

Status: Needs work » Needs review
Berdir’s picture

Status: Needs review » Reviewed & tested by the community

This now looks good for real :)

twistor’s picture

Should aggregator_category_selector be in there?

ParisLiakos’s picture

no please, aggregator_category_selector is gonna die here #15266: Replace aggregator category system with taxonomy lets not make patch there even bigger:)

twistor’s picture

Didn't even think about, just did a search for all aggregator variables. Good to go.

catch’s picture

Status: Reviewed & tested by the community » Fixed

aggregrator, whoops.

Committed/pushed to 8.x, thanks!

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