Uploaded image for project: 'CiviCRM'
  1. CiviCRM
  2. CRM-18563

Issues with the API and testing the API

    Details

    • 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.

      • 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.
      • 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")),
      ));
      

        Attachments

          Activity

            People

            • Assignee:
              mollux Mattias Michaux
              Reporter:
              mollux Mattias Michaux
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: