Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
    • Help
    • Submit feedback
    • Contribute to GitLab
  • Sign in
C
Core
  • Project
    • Project
    • Details
    • Activity
    • Releases
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
  • Issues 323
    • Issues 323
    • List
    • Board
    • Labels
    • Milestones
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Commits
  • Issue Boards
  • Development
  • Core
  • Issues
  • #676

Closed
Open
Opened Jan 22, 2019 by totten@totten
  • Report abuse
  • New issue
Report abuse New issue

CRM_Utils_GeocodeTest throwing test-negatives everywhere

Details

The relevant code is in https://github.com/civicrm/civicrm-core/blob/5.10/tests/phpunit/CRM/Utils/GeocodeTest.php

Here's an example of the test output:


Screen_Shot_2019-01-21_at_5.09.42_PM

Discussion

Generally, it looks like the tests take the approach of (a) setup example DB, (b) call Google geocoder API, (c) assert that data is updated with accurate-ish info. Some ideas to mitigate:

  1. Turn off testing of CRM_Utils_GeocodeTest completely. Communicate more loudly about the functionality moving to an extension. Perhaps bundle the extension.

  2. Keep the test, but reduce its scope/utility: in the test output, it shows geo coords like 0.00,0.00. Make these acceptable. Generally, start by nulling-out the data; then call the geocoder; then assert that the data is numerical. But don't assert that the data is specifically accurate. This utility of this depends on how we wind up with 0.00 values.

  3. Keep the test, but reduce its scope/utility: setup a dummy geocoder which uses mock data.

  4. Exclude the test from execution - but keep it in the codebase.

  5. Setup an API key for testing.

    • From security perspective, we can have the CI server pass the key as an environment variable, and the content will be automatically censored in logs. It's not exactly hacker-proof, but it provides some protection.
    • From a cost perspective, I guess it depends how much we use the api. The content of CRM_Utils_GeocodeTest doesn't look like it'll send a lot of requests. (Maybe ballpark of <10?)
    • Suppose we flag the test so that it only runs in CiviCRM-Core-Matrix. With 10 requests-per-test-run * 4 test-runs-per-day * 30.4 days-per-mo * 2 server-envs * 4 versions, then we'd be around 10k requests per month.
    • Suppose we run the test in all core PRs. If there are 200 PRs/mo and each gets 3 revisions/retests, then that's 200310 or ~6k requests per month.
    • Google currently says that they charge $5 per 1k requests; with 10k+6k requests, that's $80/mo. But they also say you get a $200/mo credit. That puts us under the budget.
    • If the capacity/requirements stay the same, then that's great. But... if somebody does a build-out on the tests to significantly improve coverage, then it's easy to imagine 5x increase in #requests, at which point the costs flip (5*$80=$400/mo), so we'd be on the hook for a balance of $200/mo. It's really not hard to imagine developers quietly increasing IO usage by 5x...
  6. Setup an API key... and also a way for these particular tests to run less often (but still feed smoothly into QA process).

Anyway, I'm holding off on judging for a moment. Just want to track this so others can feedack.

Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
None
Due date
No due date
2
Labels
Concept approved triaged
Assign labels
  • View project labels
Reference: dev/core#676