Improvements in unit tests

[HermanPeeren]

See my article in the Joomla Community Magazine about tests with PHPUnit: https://magazine.joomla.org/all-issues/october-2025/test-your-extension,-part-1-phpunit. Quality of tests is more important than test coverage. Otherwise tests will only become a maintenance burden instead of being useful.

As discussed today (22-10-2025) with Rahul I went through the unit tests and checked whether they are useful or could be improved. THIS IS STILL UNFINISHED, WORK IN PROGRESS. Will complete this review for all tests and also suggest some others.

---

✅ = keep
❌ = delete
❓ = ?
⚠️ = should be added

• Controller

  • DisplayControllerTest
    • testControllerExtendsCorrectParent
      As you can also create a valid controller that doesn't extend from BaseController, why would you limit to this implementation? ❌
    • testDefaultViewProperty
      It can be OK to change the default view to another value. Unnecessary limitation. Moreover: it is questionable to test private and protected variables and methods. ❌
      BTW you override the already existing $default_view property.
    • testControllerCanBeInstantiated
      If it cannot be instantiated the setup of $this->controller would fail. This test is not useful. ❌
  • ImportControllerTest
    • testControllerExtendsCorrectParent
      As you can also create a valid controller that doesn't extend from BaseController, why would you limit to this implementation? ❌
    • testControllerCanBeInstantiated
      If it cannot be instantiated the setup of $this->controller would fail. This test is not useful. ❌
    • testImportValidatesToken
      Although the testmethod name and the comment suggests it tests if the CSRF token is checked, the test only asserts the import() method exists and is public. If you want to assure that, then make it an interface and have the class implement that interface, instead of doing that in a test. ❌
      ⚠️ A test if the token is validated would be useful (blackbox test to see if you get an error message, or whitebox test to see if checkToken() method is called.
    • testConnectionValidatesToken
      same story as above in testImportValidatesToken, but then for the testConnection() method. ❌t an interface and have the class implement that interface, instead of doing that in a test. ❌
      ⚠️ A test if the token is validated would be useful (blackbox test to see if you get an error message, or whitebox test to see if checkToken() method is called.
    • testProgressReturnsJson
      This test doesn't test the progress file at all; it only checks that the temporary progress file that was created in this test with a valid JSON string, exists and has a valid JSON string... This is not only useless, but gives the false impression that it would be tested if the progress file returns JSON. ❌
    • testImportWithValidData
      the name of the test and description are promising, but after a long setup, nothing is tested (except for the existence of the ImportController, of which the setup would already fail if the class cannot be found. ❌
      t an interface and have the class implement that interface, instead of doing that in a test. ❌
      ⚠️ Maybe it can become ❓ a useful test if it would check the output, given valid input. And of course: after the happy path, please test for invalid input and for edge cases.

• Event

  • MigrationEventTest
    • testEventExtendsCorrectParent
      See above, other parent tests. I don't think this is useful. ❌
    • testEventCanBeInstantiated
      See above, other instantiating tests. I don't think this is useful. ❌
    • testConstructorSetsEventName
      This tests Joomla's Framework AbstractEvent, not some new functionality. Is already tested in https://github.com/joomla-framework/event/blob/3.x-dev/Tests/AbstractEventTest.php ❌
    • testConstructorAcceptsArguments
      This tests Joomla's Framework AbstractEvent, not some new functionality. Is already tested in https://github.com/joomla-framework/event/blob/3.x-dev/Tests/AbstractEventTest.php ❌
      Why do you override the constructor and the getArguments() method? Doesn't make sense to me. See issue MigrationEvent improvements #54 .
    • testConstructorWithEmptyArguments
      This tests Joomla's Framework AbstractEvent, not some new functionality. Is already tested in https://github.com/joomla-framework/event/blob/3.x-dev/Tests/AbstractEventTest.php ❌
    • testGetArgumentsReturnsCorrectData
      This tests Joomla's Framework AbstractEvent, not some new functionality. Is already tested in https://github.com/joomla-framework/event/blob/3.x-dev/Tests/AbstractEventTest.php ❌
    • testAddResult, testAddResultWithMultipleResults
      Results are stored in $arguments. You can use the Joomla\CMS\Event\ResultResultAware trait. See my code remarks about this Event. You would then be testing this interface / trait. There are no tests for addResult() yet, but if you want them they should be added to the Joomla CMS tests, not here in your derived class. ❌
    • testGetResultsReturnsEmptyArrayWhenNoResultsAdded
      Yes, if you add a getResults() method and implement it properly (see my code remarks about this event), you can test it in this way. ✅ Not sure how useful this test is ❓
    • testGetResultsReturnsArrayEvenAfterAddingNullResult
      Yes, if you add a getResults() method and implement it properly (see my code remarks about this event, issue MigrationEvent improvements #54 ), you can test it in this way. ✅ Not sure how useful this test is ❓
    • testEventArgumentsAreImmutableThroughGetArguments
      This tests Joomla's Framework AbstractEvent, not some new functionality. Is already tested in https://github.com/joomla-framework/event/blob/3.x-dev/Tests/AbstractEventTest.php ❌
    • testEventNameIsPreserved
      This tests Joomla's Framework AbstractEvent, not some new functionality. Is already tested in https://github.com/joomla-framework/event/blob/3.x-dev/Tests/AbstractEventTest.php ❌

• Extension

  • CmsMigratorComponentTest
    The whole CmsMigratorComponent can be left out; see issue Empty Extension file can be left out #53 . Those tests can be deleted.
    • testComponentExtendsCorrectParent ❌
    • testComponentImplementsBootableInterface
      ... and the BootableExtensionInterface doesn't have to be implemented at all. ❌
    • testBootMethod ❌
    • testComponentCanBeInstantiated ❌

• Model

  • ImportModelTest
    • testImportThrowsExceptionForInvalidFile ✅
      ⚠️ There might be more tings to test. Now it is only a non-existing file, but it can also be invalid JSON-format and maybe more...

TODO, not reviewed yet:

• MediaModelTest

  • testExtractImageUrlsFromContent
  • testMigrateMediaInContentReturnsEarlyForEmptyContent
  • testMigrateMediaInContentFailsConnection
  • testTestConnectionMissingFields
  • testTestConnectionValidFtp

• ProcessorModelTest

  • testProcessRoutesToCorrectMethodBasedOnDataStructure
  • testProcessThrowsExceptionForInvalidDataFormat
  • testGetOrCreateCategoryWhenCategoryExists

• Table

  • ArticleTableTest

• View/Cpanel

  • HtmlViewTest

• Integration tests with PHPUnit

  • ComponentIntegrationTest
    • testComponentCanBeInstantiatedAndInitialized
    • testImportModelEventSystemIntegration
    • testControllerModelIntegration
    • testCompleteDataFlowFromJsonToProcessing
    • testEventCreationAndResultHandling
    • testErrorHandlingThroughoutComponent
    • testFileUploadValidationIntegration
    • testMediaConfigurationIntegration
    • testComponentNamespaceAndAutoloading
    • testCompleteMigrationWorkflowSimulation
  • ComponentPackageTest
    • testCreateComponentPackage
    • addDirectoryToZip
    • verifyZipContents

Mockery in composer: is it used? Otherwise: leave it out of composer.json