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.
Follow-up #2881030-15: $total_items can be negative if pager has offset.
@Manuel Garcia:
Perhaps tackling this problem on a different issue for all core tests would make sense?
Problem/Motivation
- Better IDE supports
- Shorter (in most cases)
- We already do this when we add a new code
Proposed resolution
- Static case:
+use Drupal\block\Entity\Block; ... - $entity = $this->getMockBuilder('\Drupal\block\Entity\Block') + $entity = $this->getMockBuilder(Block::class)
- Collisions case (full path):
... use Drupal\migrate\Plugin\migrate\destination\Config; ... - $config = $this->getMockBuilder('Drupal\Core\Config\Config') + $config = $this->getMockBuilder(\Drupal\Core\Config\Config::class)
- Dynamic case (don't change):
$mock_builder = $this->getMockBuilder('Drupal\system\Plugin\ImageToolkit\Operation\gd\\' . $class_name);
Remaining tasks
- Evaluate the proposal and find out the scope
- Write a script for maximum automation of changes
- Add a rule to the Code Standards
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#11 | 2886352-11-fails.patch | 11.11 KB | Anonymous (not verified) |
#11 | 2886352-11.patch | 837 KB | Anonymous (not verified) |
#10 | 2886352-10.patch | 804.14 KB | Manuel Garcia |
#10 | interdiff.txt | 627 bytes | Manuel Garcia |
#7 | 2886352-7.patch | 804.14 KB | Manuel Garcia |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedvaplas created an issue. See original summary.
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedReproduce script:
We have 4 files with collisions of names:
Now the script use the full signature in these cases, like
Comment #3
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commentedBrilliant @vaplas!
Adding tag for visibility =)
Comment #4
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commentedApplied the patch, can still find uncovered tests in:
Almost there!
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedThanks for review and motivation, @Manuel Garcia!
Yeah, now the script worked only with files that end on
'..Test.php'
, not TestTrait.php, TestCase, etc. Good points, will be fixed.It seems we should replace only static cases. IS updated.
Also current script replaces only next methods:
Maybe something else too?
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #7
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commentedI agree about
ImageTest::getToolkitOperationMock()
.I couldn't help myself and fixed the listed remaining by hand, which thinking now about it may not have been such a great idea... anyway here it is :)
Comment #8
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commentedI can see uses still of
getMockBuilder
on:setClass
has uses still on:getMockForAbstractClass
,getMockForTrait
,prophesize
andsetExpectedException
seem to be done (yay!)Comment #10
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation commentedMissing semicolon... /me hides
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commented#7: Yeah!)
#8 good research!
Research into the possibility of replacing all strings (if they are existing classes, interfaces or traits) revealed a same discrepancy (see fails.patch).
Also, the current script has a number of shortcomings:
'use'
import)But I'll try to fix it in the future.
Comment #15
borisson_We should probably postpone this issue on #2929133: Replace all usages of getMock(), because the conflicts will be in just about every line of this patch.
Comment #17
dawehnerIs there any way how we could introduce some form of
phpcs.xml.dist
check for this?