Details
-
Type: Bug
-
Status: Done/Fixed
-
Priority: Major
-
Resolution: Fixed/Completed
-
Affects Version/s: 4.7.7
-
Component/s: CiviCRM API, Test suite
-
Labels:
-
Documentation Required?:None
-
Sprint:4.7.11 API
-
Funding Source:Contributed Code
Description
While testing CiviCRM for PHP7 compatibility, I discovered somme issues with the phpunit tests.
Most of them were discovered when outputting the PHP warnings and notices when running the tests, and looking for the cause of them.
Showing the warnings and notices is done by changing phpunit.dist.xml
- convertErrorsToExceptions="true" - convertNoticesToExceptions="true" - convertWarningsToExceptions="true" + convertErrorsToExceptions="false" + convertNoticesToExceptions="false" + convertWarningsToExceptions="false"
Some tests will fail after this, as some depend on those errors (see below).
There is something wrong with these settings, as I expected that with the original settings if there was a PHP warning or notice during a unit test, it would fail. But for some reason they aren't, which leads to a lot of wrong tests results. The problems listed below should have triggered test failures, so the cause of this should really be found.
I don't know if it is necessary to create separate issues per problem, but here is a list.
I have a (partial) solution for most of the problems, and I will provide PR's and update this post with the url's. If separate issues are necessary, I will create them.
- Documentation is not generated for new API calls, because of incorrect usage of single quotes and variables. (e.g. there is no example documentation for Note)
PR : https://github.com/civicrm/civicrm-core/pull/8374
- The generated example structure isn't consistent with API output, which leads to false positives. (Can't find an example right now).
PR : https://github.com/civicrm/civicrm-core/pull/8375
- CiviUnitTestCase->callAPIFailure doesn't pass the expected error message to CiviUnitTestCase->assertAPIFailure, which leads to non-failing tests that should fail. (e.g. the error message in api_v3_PaymentTest::testDeletePayment isn't correct, but the test passes)
PR: https://github.com/civicrm/civicrm-core/pull/8376
- some tests trigger php warnings, but those are currently ignored, as they aren't visible and the tests pass.
- test CRM_Contact_Form_Search_Custom_SampleTest::testAll falsely fails when testing on mariadb, because the default sort order is different in MySQL and MariaDB.
PR: https://github.com/civicrm/civicrm-core/pull/8377
- Some tests depend on a specific PHP warnings, which IMO isn't a good idea, e.g. api_v3_CustomGroupTest::testCustomGroupExtendsMultipleCreate.
- api_v3_SyntaxConformanceTest::testWithoutParam_create checks if a PHPUnit_Framework_Error ( = PHP warning, notice or deprecated) is triggered, but it should check if there is an API error (e.g. creating a PrintLabel without a name or title should throw an error). This isn't the case, so test don't fail and it is possible to create a LocBlock, MailingComponent, Mapping, Navigation, PrintLabel, RecurringEntity, SavedSearch, Setting, WordReplacement, GroupContact and Profile without any params.
PR : https://github.com/civicrm/civicrm-core/pull/8379
- IMO, currently api_v3_SyntaxConformanceTest::testWithoutParam_get has no use, as it tests if a php warning is triggered, which is always the case.
- CRM_Contribute_DAO_Product has field named 'options', which conflicts with the options param in an API call. This throws errors in civicrm_api3_generic_getfields. I guess there should be a blacklist of possible field names for BAO's exposed through the API?
- _civicrm_api_get_custom_fields and civicrm_api3_generic_getfields and can't handle multi-value contact_type param when calling e.g.
$result = civicrm_api3('Contact', 'get', array( 'sequential' => 1, 'contact_type' => array("Household", "Individual"), )); $result = civicrm_api3('Contact', 'get', array( 'sequential' => 1, 'contact_type' => array('IN' => array("Household", "Individual")), ));
- Some tests output debug info which isn't necessary, and it isn't convenient when running the tests without --tap
PR: https://github.com/civicrm/civicrm-core/pull/8378
- API permission check doesn't show 'or' permissions
PR : https://github.com/civicrm/civicrm-core/pull/8380
Attachments
Issue Links
- links to