test(#3432653): cover the migration path that reported the crash - #26
test(#3432653): cover the migration path that reported the crash#26Decipher wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds kernel regression coverage for migrated media files and unregistered temporary stream-wrapper schemes. Tests verify successful migration, file processing, destination creation, and avoidance of fatal errors. ChangesMedia file processing tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds regression coverage for the migration path and unregistered temporary URI handling without changing production behavior. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.x-1.x #26 +/- ##
===========================================
- Coverage 83.39% 83.37% -0.03%
===========================================
Files 20 20
Lines 783 782 -1
===========================================
- Hits 653 652 -1
Misses 130 130 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
02bc7cf to
84a5030
Compare
Refs https://www.drupal.org/project/filefield_paths/issues/3432653
Summary
Test-only. The reported fatal is already fixed by the object oriented conversion; this adds the regression coverage that was never written.
Why there is no fix here
The crash was
Call to a member function getType() on boolatfilefield_paths.inc:75. The old code resolved stream wrappers withStreamWrapperManager::getViaUri(...)->getType(), andgetViaUri()returns FALSE for a scheme no wrapper handles. That file is now a deprecation stub that nothing loads, and the replacement never calls a method on a wrapper lookup: it parses the scheme and guards withempty()before use.Verification
Both reporters hit this during a migration, so the coverage goes in at that layer rather than by calling the service directly:
MigrateMediaFileTestruns a realMigrateExecutableover anembedded_datasource intoentity:media, so the hook fires fromhook_entity_insertinside a migration, and asserts the import returnsRESULT_COMPLETED.temp_locationto an unregistered scheme, which is the reporter's environment, and asserts up front that the scheme really has no wrapper so the test cannot pass vacuously.FileFieldPathsProcessFileLegacyTest::testUnregisteredTemporarySchemeIsSkippedcovers the same input at service level.Summary by CodeRabbit