Spun off from #2554321: Clean up Migrate's test suite.
MigrateDrupalTestBase contains a method, testSourcePlugin(), which is absolutely misplaced. It was only ever intended as a regression test against a really dumb bug which would have been caught if all source plugins had accompanying unit tests (most do, but a few don't). When #2554321: Clean up Migrate's test suite lands, each test method in Migrate Drupal's integration tests will incur a considerable amount of overhead due to all the required migrations running in setUp(), and this method really does not belong in the integration test suite anyway.
testSourcePlugin() should be removed and replaced with dedicated unit tests for all source plugins which use DummyQueryTrait. These will be able to catch the original regression, and provide a place to add further tests of each plugin if needed.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2573559-11.patch | 38.31 KB | phenaproxima |
| #6 | interdiff-2573559-6.txt | 303 bytes | phenaproxima |
| #6 | interdiff-2573559-5-6.txt | 1.83 KB | phenaproxima |
| #6 | 2573559-6-FAIL.patch | 38.14 KB | phenaproxima |
| #6 | 2573559-6.patch | 37.63 KB | phenaproxima |
Comments
Comment #2
phenaproximaFixing an errant character in the issue title.
Comment #3
mikeryanThat's kinda backwards - if a plugin is using DummyQueryTrait, then it's going to be fine and won't exhibit the original bug. The point of testSourcePlugin() is to automatically catch if a new source plugin is added which has a bogus query() and/or count() method, and should be using DummyQueryTrait.
Comment #4
phenaproximaComment #5
phenaproximaSo, first off, I apologize for the unexpected length of this patch.
This ices MigrateDrupalTestBase::testSourcePlugin() -- good riddance, IMO. That, however, required me to add tests for source plugins which didn't already have unit tests. That was the point of testSourcePlugin() to begin with -- a way to work around the holes in our source plugin test coverage. I've moved its assertions into MigrateSqlSourceTest and added a lot of missing tests.
This should, hopefully, be pretty non-controversial -- after all, it adds a lot of new test coverage to Migrate Drupal.
Comment #6
phenaproximaHere is the witch-dunker -- the fail patch reintroduces a buggy condition which is caught by the new UserPictureInstanceTest.
Comment #11
phenaproximaBah, humbug!
Comment #14
mikeryanGave up waiting on PIFR, RTBC...
Comment #15
webchickLove the smell of fresh tests in the morning. ;)
Committed and pushed to 8.0.x. Thanks!
Comment #18
mikeryan