Use a data factory for Phabricator mock responses

Using carefully-crafted test data fixtures can lead to brittle tests and
difficult-to-construct test scenarios in our integration tests.

Switch the test suite to use a data factory instead of static fixtures
to mock Phabricator responses.

Author: mars-f

landoapi/utils.py

@@ -0,0 +1,23 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+
+def extract_rawdiff_id_from_uri(uri):
+    """Extract a raw diff ID from a Diff uri."""
+    # The raw diff is part of a URI, such as
+    # "https://secure.phabricator.com/differential/diff/43480/".
+    parts = uri.rsplit('/', 4)
+
+    # Check that the URI Path is something we understand.  Fail if the
+    # URI path changed (signalling that the raw diff part of the URI may
+    # be in a different segment of the URI string).
+    if parts[1:-2] != ['differential', 'diff']:
+        raise RuntimeError(
+            "Phabricator Raw Diff URI parsing error: The "
+            "URI {} is not in a format we "
+            "understand!".format(uri)
+        )
+
+    # Take the second-last member because of the trailing slash on the URL.
+    return int(parts[-2])

tests/canned_responses/phabricator/revisions.py

@@ -78,7 +78,7 @@
     "error_info": None,
 }
 
-CANNED_REVISION_EMPTY = {
+CANNED_EMPTY_RESULT = {
     "result": [],
     "error_code": None,
     "error_info": None

tests/conftest.py

@@ -1,12 +1,48 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
+import json
 
 import pytest
+import requests_mock
+
+from landoapi.app import create_app
+from tests.factories import PhabResponseFactory
 
 
 @pytest.fixture
 def docker_env_vars(monkeypatch):
     """Monkeypatch environment variables that we'd get running under docker."""
     monkeypatch.setenv('PHABRICATOR_URL', 'http://phabricator.test')
     monkeypatch.setenv('TRANSPLANT_URL', 'http://autoland.test')
+
+
+@pytest.fixture
+def phabfactory():
+    """Mock the Phabricator service and build fake response objects."""
+    with requests_mock.mock() as m:
+        yield PhabResponseFactory(m)
+
+
+@pytest.fixture
+def versionfile(tmpdir):
+    """Provide a temporary version.json on disk."""
+    v = tmpdir.mkdir('app').join('version.json')
+    v.write(
+        json.dumps(
+            {
+                'source': 'https://github.com/mozilla-conduit/lando-api',
+                'version': '0.0.0',
+                'commit': '',
+                'build': 'test',
+            }
+        )
+    )
+    return v
+
+
+@pytest.fixture
+def app(versionfile):
+    """Needed for pytest-flask."""
+    app = create_app(versionfile.strpath)
+    return app.app

tests/factories.py

@@ -0,0 +1,129 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+"""
+Data factories for writing integration tests.
+"""
+from copy import deepcopy
+
+from landoapi.utils import extract_rawdiff_id_from_uri
+from tests.canned_responses.phabricator.repos import CANNED_REPO_MOZCENTRAL
+from tests.canned_responses.phabricator.revisions import CANNED_EMPTY_RESULT, \
+    CANNED_REVISION_1, CANNED_REVISION_1_DIFF, CANNED_REVISION_1_RAW_DIFF
+from tests.canned_responses.phabricator.users import CANNED_USER_1
+from tests.utils import phab_url, first_result_in_response, phid_for_response, \
+    form_matcher
+
+
+class PhabResponseFactory:
+    """Mock Phabricator service responses with generated data."""
+
+    def __init__(self, requestmocker):
+        """
+        Args:
+            requestmocker: A requests_mock.mock() object.
+        """
+        self.mock = requestmocker
+        self.mock_responses = {}
+        self.install_404_responses()
+
+    def install_404_responses(self):
+        """Install catch-all 404 response handlers for API queries."""
+        query_urls = ['differential.query', 'phid.query', 'user.query']
+        for query_url in query_urls:
+            self.mock.get(
+                phab_url(query_url), status_code=404, json=CANNED_EMPTY_RESULT
+            )
+
+    def user(self):
+        """Return a Phabricator User."""
+        user = deepcopy(CANNED_USER_1)
+        self.mock.get(phab_url('user.query'), status_code=200, json=user)
+        return user
+
+    def revision(self, **kwargs):
+        """Return a Phabricator Revision."""
+        result_json = deepcopy(CANNED_REVISION_1)
+        revision = first_result_in_response(result_json)
+
+        if 'id' in kwargs:
+            # Convert 'D000' form to just '000'.
+            str_id = kwargs['id']
+            num_id = str_id[1:]
+            revision['id'] = num_id
+            revision['phid'] = "PHID-DREV-%s" % num_id
+
+        if 'depends_on' in kwargs:
+            parent_revision_response_data = kwargs['depends_on']
+            if parent_revision_response_data:
+                # This Revisions depends on another Revision.
+                new_value = [phid_for_response(parent_revision_response_data)]
+            else:
+                # The user passed in None or an empty list, saying "this
+                # revision has no parent revisions."
+                new_value = []
+            revision['auxiliary']['phabricator:depends-on'] = new_value
+
+        # Revisions have at least one Diff.
+        diff = self.diff()
+        diffID = extract_rawdiff_id_from_uri(
+            first_result_in_response(diff)['uri']
+        )
+        rawdiff = self.rawdiff(diffID=str(diffID))
+        revision['activeDiffPHID'] = phid_for_response(diff)
+
+        # Revisions may have a Repo.
+        repo = self.repo()
+        revision['repositoryPHID'] = phid_for_response(repo)
+
+        def match_revision(request):
+            # Revisions can be looked up by PHID or ID.
+            found_phid = form_matcher('phids[]', revision['phid'])(request)
+            found_id = form_matcher('ids[]', revision['id'])(request)
+            return found_phid or found_id
+
+        self.mock.get(
+            phab_url('differential.query'),
+            status_code=200,
+            json=result_json,
+            additional_matcher=match_revision
+        )
+
+        # Revisions can also be looked up by phid.query.
+        self.phid(result_json)
+
+        return result_json
+
+    def diff(self):
+        """Return a Revision Diff."""
+        diff = deepcopy(CANNED_REVISION_1_DIFF)
+        self.phid(diff)
+        return diff
+
+    def rawdiff(self, diffID='12345'):
+        """Return raw diff text for a Revision Diff."""
+        rawdiff = deepcopy(CANNED_REVISION_1_RAW_DIFF)
+        self.mock.get(
+            phab_url('differential.getrawdiff'),
+            status_code=200,
+            json=rawdiff,
+            additional_matcher=form_matcher('diffID', diffID)
+        )
+        return rawdiff
+
+    def repo(self):
+        """Return a Phabricator Repo."""
+        repo = deepcopy(CANNED_REPO_MOZCENTRAL)
+        self.phid(repo)
+        return repo
+
+    def phid(self, response_data):
+        """Add a phid.query matcher for the given Phabricator response object.
+        """
+        phid = phid_for_response(response_data)
+        self.mock.get(
+            phab_url('phid.query'),
+            status_code=200,
+            additional_matcher=form_matcher('phids[]', phid),
+            json=response_data
+        )

tests/test_dockerflow.py

@@ -4,8 +4,6 @@
 
 import json
 
-from tests.utils import versionfile, app
-
 
 def test_dockerflow_lb_endpoint_returns_200(client):
     assert client.get('/__lbheartbeat__').status_code == 200

tests/test_phabricator_client.py

@@ -9,6 +9,7 @@
 
 from landoapi.phabricator_client import PhabricatorClient, \
     PhabricatorAPIException
+from landoapi.utils import extract_rawdiff_id_from_uri
 
 from tests.utils import *
 from tests.canned_responses.phabricator.revisions import *
@@ -80,3 +81,16 @@ def test_phabricator_exception():
             phab.get_revision(id=CANNED_REVISION_1['result'][0]['id'])
         assert e_info.value.error_code == CANNED_ERROR_1['error_code']
         assert e_info.value.error_info == CANNED_ERROR_1['error_info']
+
+
+def test_extracting_rawdiff_id_from_properly_formatted_uri():
+    # Raw diff ID is '43480'
+    uri = "https://secure.phabricator.com/differential/diff/43480/"
+    rawdiff_id = extract_rawdiff_id_from_uri(uri)
+    assert rawdiff_id == 43480
+
+
+def test_raises_error_if_rawdiff_uri_segments_change():
+    uri = "https://secure.phabricator.com/differential/SOMETHINGNEW/43480/"
+    with pytest.raises(RuntimeError):
+        extract_rawdiff_id_from_uri(uri)