Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
The errorCount method on the MigrateIdMapInterface documents the return value as a string but Sql::errorCount() returns a string. This is likely true for the other count methods, importedCount, UpdateCount, messageCount, processedCount. Note that NullIdMap::errorCount(), and the other count methods in that class, return an integer.
Assertions in MigrateSqlIdMapTest.php are aware of this
$this->assertSame(2, (int) $id_map->importedCount());
Proposed resolution
Change the methods in Sql to return an int
Remaining tasks
Write a patch
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 2937666-5.patch | 5.8 KB | jofitz |
| #5 | interdiff-2-5.txt | 577 bytes | jofitz |
| #2 | 2937666-2-complete.patch | 5.24 KB | jofitz |
| #2 | 2937666-2-test_only.patch | 3.95 KB | jofitz |
Comments
Comment #2
jofitz CreditAttribution: jofitz at ComputerMinds commentedTighten up the tests and cast the results of the 5 count functions.
Comment #5
jofitz CreditAttribution: jofitz at ComputerMinds commentedCorrect test failure. It seems other places are promising integers and returning strings too!
Comment #6
quietone CreditAttribution: quietone as a volunteer commentedThanks, that tidies this up nicely. I applied the patch and grepped for all the count functions and found one odd one. Why did this not require a change?
In MigrateFieldTest.php
$this->assertSame($migration->getSourcePlugin()->count(), $migration->getIdMap()->processedCount());Comment #7
jofitz CreditAttribution: jofitz at ComputerMinds commented@quietone The instance you have highlighted did not require a change because previously it was comparing to integer strings, it is now comparing two integers. getSourcePlugin() uses the count() function in SqlBase.php (that has been updated to cast to integer) while getIdMap() uses the processedCount() function in Sql.php (that has also been updated to cast to integer).
Comment #8
quietone CreditAttribution: quietone as a volunteer commented@Jo Fitzgerald, thanks. I see it now.
Comment #10
catchCommitted/pushed to 8.6.x and cherry-picked to 8.5.x. Thanks!