The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
@longwave found a possible error at one of the tests from core/modules/node/tests/src/Kernel/Plugin/migrate/source/d7/NodeTest.php (see here)
The tests [0] and [5] are the same.
Steps to reproduce
Open the file.
Proposed resolution
TODO
Remaining tasks
- Figure out why the code is like this.
- Propose a solution
- Fix it
- Review
- Commit
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | reroll_diff_3285667_5-7.txt | 1020 bytes | ankithashetty |
| #7 | 3285667-7.patch | 6.02 KB | ankithashetty |
| #5 | interdiff_3285667_2-5.txt | 4.89 KB | andregp |
| #5 | 3285667-5.patch | 6.03 KB | andregp |
| #2 | 3285667-2.patch | 1.06 KB | quietone |
Comments
Comment #2
quietone commentedThe test was introduced in [#293798915]. It does appear to be a duplicate of $test[0]. I don't think it was intended to test something else, it is just an oversight. The test is testing without a configuration key 'node_type', with it set to 'page' and with it set to 'article' and 'page'. That should be sufficient for testing the 'node_type' key.
I have removed $test[5] and moved that comment from that test to $test[0]
Comment #3
quietone commentedFix typo in title
Comment #4
longwaveRemoving this fixes a PHPStan error, so we have to remove that from the baseline as well.
Comment #5
andregp commentedRemoved the PHPStan ignore line related to this.
Comment #6
longwaveComment #7
ankithashettyUploading a rerolled patch, thanks!
Comment #8
longwaveThanks. I agree with @quietone's assessment in #2 that this was an oversight in the original patch and the extra test case is unnecessary.
Comment #10
spokjeRandom JS Test failure, started retest to be sure and back to RTBC.
Comment #13
catchCommitted/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!
Doesn't cherry-pick to 9.5.x due to phpstan baseline, but I think that's fine.