Fix for issue #17901 - improvements in fragment handling and unittesting #17954
Conversation
BEFICENT
commented
Oct 8, 2024
BEFICENT
commented
| if ($name[0] == '/') { | ||
| $path = "..{$name}"; | ||
| } else { | ||
| $path = "..{$this->name}/{$name}"; |
There was a problem hiding this comment.
Using the $this->name is not correct IMO. For e.g. if the current controller name is Admins but the template path is set to Users then I would expect it to look for the fragment under Users folder.
The View class is also used for rendering emails and cell where the name property is irrelevant/empty.
| "phpunit/phpunit": "^10.5.5 || ^11.1.3" | ||
| "phpstan/phpstan": "1.12.3", | ||
| "phpunit/phpunit": "^10.5.5 || ^11.1.3", | ||
| "squizlabs/php_codesniffer": "^3.10", |
There was a problem hiding this comment.
Can we omit this. We don't need to change dependencies to add view methods.
| ->getMock(); | ||
|
|
||
| // Set up the element method to return the stored fragment data | ||
| $this->View->expects($this->any()) |
There was a problem hiding this comment.
I would avoid using mocks for this. Instead add more templates to the test application and use those for your tests. Our test suite is more reliable and easier to understand if we use fewer mocks and have more 'integration tests' that use the framework like applications would.
|
|
||
| // Verify that the fragment is cleared by checking the stored fragment | ||
| $clearedResult = $this->View->fragment('testFragment'); | ||
| echo 'Cleared fragment: ' . $clearedResult . "\n"; |
There was a problem hiding this comment.
Tests shouldn't be generating output.
