User tests: Successful: Unsuccessful:
Pull Request for Issue # .
In none static methods of component controllers, we should use $this->app to get application object instead of using Factory::getApplication(). This PR just makes that change.
I hope code review would be enough. Otherwise, I will have to split this simple PR to multiple small PRs for real user testing.
Works !
Works !
None
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_cpanel com_finder com_languages Front End com_contact com_content |
Labels |
Added:
?
Maintainers Checked
|
Restarted drone. Wondering if the system test installation is failing because of the pr or just a hickup.
Wondering about the same. But I haven't found anything in the log or the screenshot from the test results telling what it could be.
The restart from yesterday and the branch update now were both successfully.
The restart from yesterday and the branch update now were both successfully.
Maybe it depends on the moon phase
I have tested this item
I really appreciate all the work you are and @laoneo are doing but I really think that it is far too late in the release process of 4.2 to be making these changes.
I think this is up to the release lead to decide. And nobody says that this needs to go into 4.2. But thats the branch where this kind of changes should be done to atm.
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-06-27 15:49:48 |
Closed_By | ⇒ | HLeithner |
Thanks
Could anyone tell me why do we have to make this change? And Factory::getApplication()
will be bad practice ?
So inside the controller we want to take better advantage of dependency injection. This allows us to create significantly better unit tests over the controllers and therefore improve both the quantity and quality of tests by creating mocks. Additionally injecting like this allows us to take better advantage of the Dependency injection container.
The static factory settings conversely are more difficult to mock across multiple tests, mean its harder to utilise the DIC. Furthermore by making this dynamic it improves our ability to use the same controller in different applications (as API starts to become more of a first class citizen this will become more and more clear I think)
Restarted drone. Wondering if the system test installation is failing because of the pr or just a hickup.