diff --git a/hgext3rd/arcdiff.py b/hgext3rd/arcdiff.py --- a/hgext3rd/arcdiff.py +++ b/hgext3rd/arcdiff.py @@ -13,7 +13,7 @@ from phabricator import ( arcconfig, - conduit, + graphql, diffprops, ) @@ -21,23 +21,21 @@ entry = extensions.wrapcommand(commands.table, 'diff', _diff) options = entry[1] options.append(('', 'since-last-arc-diff', None, - _('show changes since last `arc diff`'))) + _('show changes since last `arc diff`'))) def _differentialhash(ui, repo, phabrev): timeout = repo.ui.configint('ssl', 'timeout', 5) ca_certs = repo.ui.configpath('web', 'cacerts') - client = conduit.Client(ca_certs=ca_certs, timeout=timeout) try: - client.apply_arcconfig(arcconfig.load_for_path(repo.root)) + client = graphql.Client( + repodir=repo.root, ca_bundle=ca_certs, repo=repo) + info = client.getrevisioninfo(timeout, [phabrev]).get(str(phabrev)) + if not info: + return None + return info - diffid = diffprops.getcurrentdiffidforrev(client, phabrev) - if diffid is None: - return None - - localcommits = diffprops.getlocalcommitfordiffid(client, diffid) - return localcommits.get('commit', None) if localcommits else None - except conduit.ClientError as e: - ui.warn(_('Error calling conduit: %s\n') % str(e)) + except graphql.ClientError as e: + ui.warn(_('Error calling graphql: %s\n') % str(e)) return None except arcconfig.ArcConfigError as e: raise error.Abort(str(e)) @@ -55,11 +53,12 @@ raise error.Abort(mess) rev = _differentialhash(ui, repo, phabrev) - if rev is None: + + if rev is None or not isinstance(rev, dict) or "hash" not in rev: mess = _('unable to determine previous changeset hash') raise error.Abort(mess) - rev = str(rev) + rev = str(rev['hash']) opts['rev'] = [rev] # if patterns aren't provided, restrict diff to files in both changesets diff --git a/hgext3rd/phabstatus.py b/hgext3rd/phabstatus.py --- a/hgext3rd/phabstatus.py +++ b/hgext3rd/phabstatus.py @@ -9,11 +9,12 @@ from mercurial import util as hgutil from mercurial.i18n import _ from mercurial import obsutil +import os from phabricator import ( arcconfig, - conduit, diffprops, + graphql, ) def memoize(f): @@ -61,7 +62,6 @@ repo.ui.warn(msg) return ["Error"] * len(diffids) - @memoize def getdiffstatus(repo, *diffid): """Perform a Conduit API call to get the diff status @@ -74,29 +74,28 @@ ca_certs = repo.ui.configpath('web', 'cacerts') try: - resp = conduit.call_conduit('differential.querydiffhashes', - {'revisionIDs': diffid}, - timeout=timeout, ca_certs=ca_certs) + client = graphql.Client( + repodir=os.getcwd(), ca_bundle=ca_certs, repo=repo) + statuses = client.getrevisioninfo(timeout, diffid) - except conduit.ClientError as ex: + except graphql.ClientError as ex: msg = _('Error talking to phabricator. No diff information can be ' 'provided.\n') hint = _("Error info: %s\n") % str(ex) - return _fail(repo, diffid, msg, hint) + ret = _fail(repo, diffid, msg, hint) + return ret except arcconfig.ArcConfigError as ex: msg = _('arcconfig configuration problem. No diff information can be ' 'provided.\n') hint = _("Error info: %s\n") % str(ex) - return _fail(repo, diffid, msg, hint) + ret = _fail(repo, diffid, msg, hint) + return ret - if not resp: - resp = {} - - # This makes the code more robust in case conduit does not return - # what we need + # This makes the code more robust in case we don't learn about any + # particular revision result = [] for diff in diffid: - matchingresponse = resp.get(diff) + matchingresponse = statuses.get(str(diff)) if not matchingresponse: result.append("Error") else: @@ -184,8 +183,7 @@ if local == remote: return "sync" elif count == 1: - precursors = list(obsutil.allpredecessors(repo.obsstore, - [ctx.node()])) + precursors = list(obsutil.allpredecessors(repo.obsstore, [ctx.node()])) hashes = [repo.unfiltered()[h].hex() for h in precursors] # hashes[0] is the current # hashes[1] is the previous diff --git a/phabricator/arcconfig.py b/phabricator/arcconfig.py --- a/phabricator/arcconfig.py +++ b/phabricator/arcconfig.py @@ -16,7 +16,7 @@ class ArcConfigError(Exception): pass -def _load_file(filename): +def _loadfile(filename): try: with open(filename, 'r') as f: return json.loads(f.read()) @@ -25,7 +25,7 @@ return None raise -def load_for_path(path): +def loadforpath(path): # location where `arc install-certificate` writes .arcrc if os.name == 'nt': envvar = 'APPDATA' @@ -36,13 +36,13 @@ raise ArcConfigError('$%s environment variable not found' % envvar) # Use their own file as a basis - userconfig = _load_file(os.path.join(homedir, '.arcrc')) or {} + userconfig = _loadfile(os.path.join(homedir, '.arcrc')) or {} # Walk up the path and augment with an .arcconfig if we find it, # terminating the search at that point. path = os.path.abspath(path) while len(path) > 1: - config = _load_file(os.path.join(path, '.arcconfig')) + config = _loadfile(os.path.join(path, '.arcconfig')) if config is not None: userconfig.update(config) # Return the located path too, as we need this for figuring @@ -57,7 +57,7 @@ def debugarcconfig(ui, repo, *args, **opts): """ exists purely for testing and diagnostic purposes """ try: - config = load_for_path(repo.root) + config = loadforpath(repo.root) ui.write(json.dumps(config, sort_keys=True), '\n') except ArcConfigError as ex: raise error.Abort(str(ex)) diff --git a/phabricator/conduit.py b/phabricator/conduit.py deleted file mode 100644 --- a/phabricator/conduit.py +++ /dev/null @@ -1,204 +0,0 @@ -# conduit.py -# -# A library function to call a phabricator conduit RPC. -# It's different from fbconduit in that this is an authenticated -# conduit client. - -from __future__ import absolute_import - -import contextlib -import hashlib -import json -import os -import time -import warnings - -import urllib3 - -from mercurial import util - -from . import arcconfig - -urlreq = util.urlreq - -DEFAULT_URL = 'https://phabricator.intern.facebook.com/api/' -DEFAULT_TIMEOUT = 60 -mocked_responses = None - -class ClientError(Exception): - def __init__(self, code, msg): - Exception.__init__(self, msg) - self.code = code - -class Client(object): - def __init__(self, url=None, user=None, cert=None, act_as=None, - ca_certs=None, timeout=None): - self._url = url or DEFAULT_URL - self._user = user - self._cert = cert - self._oauth = None - self._actas = act_as or self._user - self._connection = None - self._ca_certs = ca_certs - self._timeout = timeout - - def apply_arcconfig(self, config): - self._url = config.get('conduit_uri', DEFAULT_URL) - if self._url == 'https://phabricator.fb.com/api/': - self._url = 'https://phabricator.intern.facebook.com/api/' - try: - hostconfig = config['hosts'][self._url] - self._user = hostconfig['user'] - if 'oauth' in hostconfig: - self._oauth = hostconfig['oauth'] - else: - self._cert = hostconfig['cert'] - except KeyError: - try: - hostconfig = config['hosts'][config['hosts'].keys()[0]] - self._user = hostconfig['user'] - if 'oauth' in hostconfig: - self._oauth = hostconfig['oauth'] - else: - self._cert = hostconfig['cert'] - except KeyError: - raise arcconfig.ArcConfigError( - 'arcrc is missing user ' - 'credentials for host %s. use ' - '"arc install-certificate" to fix.' % self._url) - self._actas = self._user - self._connection = None - - def call(self, method, args, timeout=None): - if timeout is None: - if self._timeout is None: - timeout = DEFAULT_TIMEOUT - else: - timeout = self._timeout - args['__conduit__'] = { - 'authUser': self._user, - 'actAsUser': self._actas, - 'caller': 'hg', - } - if self._oauth is not None: - args['__conduit__']['accessToken'] = self._oauth - else: - token = '%d' % time.time() - sig = token + self._cert - args['__conduit__'].update({ - 'authToken': token, - 'authSignature': hashlib.sha1(sig.encode('utf-8')).hexdigest() - }) - req_data = { - 'params': json.dumps(args), - 'output': 'json', - } - headers = ( - ('Connection', 'Keep-Alive'), - ) - url = self._url + method - - if self._connection is None: - self._connection = urllib3.PoolManager(ca_certs=self._ca_certs) - try: - with warnings.catch_warnings(): - if not self._ca_certs: - # ignore the urllib3 certificate verification warnings - warnings.simplefilter( - 'ignore', urllib3.exceptions.InsecureRequestWarning) - response = self._connection.request( - 'POST', url, headers=headers, fields=req_data, - timeout=timeout) - except urllib3.exceptions.HTTPError as ex: - errno = -1 - if ex.args and util.safehasattr(ex.args[0], 'errno'): - errno = ex.args[0].errno - raise ClientError(errno, str(ex)) - - try: - response = json.loads(response.data) - except ValueError: - # Can't decode the data, not valid JSON (html error page perhaps?) - raise ClientError(-1, 'did not receive a valid JSON response') - - if response['error_code'] is not None: - raise ClientError(response['error_code'], response['error_info']) - return response['result'] - -class MockClient(object): - def __init__(self, **kwargs): - pass - - def apply_arcconfig(self, config): - pass - - def call(self, method, args, timeout=DEFAULT_TIMEOUT): - global mocked_responses - - cmd = json.dumps([method, args], sort_keys=True) - try: - response = mocked_responses.pop(0) - # Check expectations via a deep compare of the json representation. - # We need this because child objects and values are compared by - # address rather than value. - expect = json.dumps(response.get('cmd', None), sort_keys=True) - if cmd != expect: - raise ClientError(None, - 'mock mismatch got %s expected %s' % ( - cmd, expect)) - if 'error_info' in response: - raise ClientError(response.get('error_code', None), - response['error_info']) - return response['result'] - except IndexError: - raise ClientError(None, - 'No more mocked responses available for call to %s' % cmd) - - -if 'HG_ARC_CONDUIT_MOCK' in os.environ: - # To facilitate testing, we replace the client object with this - # fake implementation that returns responses from a file that - # contains a series of json serialized object values. - with open(os.environ['HG_ARC_CONDUIT_MOCK'], 'r') as f: - mocked_responses = json.load(f) - Client = MockClient - -class ClientCache(object): - def __init__(self): - self.max_idle_seconds = 10 - self.client = {} - self.lastuse = {} - - @contextlib.contextmanager - def getclient(self, ca_certs=None): - # Use the existing client if we have one and it hasn't been idle too - # long. - # - # We reconnect if we have been idle for too long just in case the - # server might have closed our connection while we were idle. (We - # could potentially check the socket for readability, but that might - # still race with the server currently closing our socket.) - client = self.client.get(ca_certs), - lastuse = self.lastuse.get(ca_certs, 0) - if client and time.time() <= (lastuse + self.max_idle_seconds): - # Remove self.client for this ca_certs config while we are using - # it. If our caller throws an exception during the yield this - # ensures that we do not continue to use this client later. - del self.client.pop[ca_certs], self.lastuse[ca_certs] - else: - # We have to make a new connection - client = Client(ca_certs=ca_certs) - client.apply_arcconfig(arcconfig.load_for_path(os.getcwd())) - - yield client - - # Our caller used this client successfully and did not throw an - # exception. Store it to use again next time getclient() is called. - self.lastuse[ca_certs] = time.time() - self.client[ca_certs] = client - -_clientcache = ClientCache() - -def call_conduit(method, args, ca_certs=None, timeout=DEFAULT_TIMEOUT): - with _clientcache.getclient(ca_certs=ca_certs) as client: - return client.call(method, args, timeout=timeout) diff --git a/phabricator/diffprops.py b/phabricator/diffprops.py --- a/phabricator/diffprops.py +++ b/phabricator/diffprops.py @@ -1,5 +1,4 @@ import re -from operator import itemgetter diffrevisionregex = re.compile('^Differential Revision:.*/D(\d+)', re.M) @@ -12,46 +11,3 @@ """ match = diffrevisionregex.search(description) return match.group(1) if match else None - -def getcurrentdiffidforrev(client, phabrev): - """Given a revision number (the 123 from D123), returns the current - diff id associated with that revision. """ - - res = client.call('differential.query', {'ids': [phabrev]}) - if not res: - return None - - info = res[0] - if not info: - return None - - diffs = info.get('diffs', []) - if not diffs: - return None - - return max(diffs) - -def getlocalcommitfordiffid(client, diffid): - """Returns the most recent local:commits entry for a phabricator diff_id""" - - res = client.call('differential.getdiffproperties', { - 'diff_id': diffid, - 'names': ['local:commits']}) - if not res: - return None - - localcommits = res.get('local:commits', {}) - if not localcommits: - return None - - # Order with most recent commit time first. A more completely correct - # implementation would toposort based on the parents properties, however, - # wez thinks that we should only contain a single entry most of the time, - # and our best prior art used to just take the first item that showed up - # in the dictionary. Sorting gives us some determinism, so we will at - # least be consistently wrong if we're wrong. - localcommits = sorted(localcommits.values(), - key=itemgetter('time'), reverse=True) - - return localcommits[0] - diff --git a/phabricator/graphql.py b/phabricator/graphql.py new file mode 100644 --- /dev/null +++ b/phabricator/graphql.py @@ -0,0 +1,156 @@ +# graphql.py +# +# A library function to call a phabricator graphql RPC. +# This replaces the Conduit methods + +import json +import os +from mercurial import util +import arcconfig +from operator import itemgetter +import phabricator_graphql_client +import phabricator_graphql_client_urllib + +urlreq = util.urlreq + +class ClientError(Exception): + def __init__(self, code, msg): + Exception.__init__(self, msg) + self.code = code + +class Client(object): + def __init__(self, repodir=os.getcwd(), ca_bundle=None, repo=None): + self._mock = 'HG_ARC_CONDUIT_MOCK' in os.environ + if self._mock: + with open(os.environ['HG_ARC_CONDUIT_MOCK'], 'r') as f: + self._mocked_responses = json.load(f) + + self._host = None + self._user = None + self._cert = None + self._oauth = None + self.ca_bundle = ca_bundle or True + self._applyarcconfig(arcconfig.loadforpath(repodir)) + if not self._mock: + app_id = repo.ui.config('phabricator', 'graphql_app_id') + app_token = repo.ui.config('phabricator', 'graphql_app_token') + self._host = repo.ui.config('phabricator', 'graphql_host') + self._client = phabricator_graphql_client.PhabricatorGraphQLClient( + phabricator_graphql_client_urllib. + PhabricatorGraphQLClientRequests(), self._cert, self._oauth, + self._user, 'phabricator', self._host, app_id, app_token) + + def _applyarcconfig(self, config): + self._host = config.get('graphql_uri', self._host) + if 'OVERRIDE_GRAPHQL_URI' in os.environ: + self._host = os.environ['OVERRIDE_GRAPHQL_URI'] + try: + hostconfig = config['hosts'][self._host] + self._user = hostconfig['user'] + self._cert = hostconfig.get('cert', None) + self._oauth = hostconfig.get('oauth', None) + except KeyError: + try: + hostconfig = config['hosts'][config['hosts'].keys()[0]] + self._user = hostconfig['user'] + self._cert = hostconfig.get('cert', None) + self._oauth = hostconfig.get('oauth', None) + except KeyError: + pass + + if self._cert is None and self._oauth is None: + raise arcconfig.ArcConfigError( + 'arcrc is missing user ' + 'credentials for host %s. use ' + '"arc install-certificate" to fix.' % self._host) + + def _normalizerevisionnumbers(self, *revision_numbers): + rev_numbers = [] + if isinstance(revision_numbers, str): + return [int(revision_numbers)] + for r in revision_numbers: + if isinstance(r, list) or isinstance(r, tuple): + for rr in r: + rev_numbers.extend(rr) + else: + rev_numbers.append(int(r)) + return [int(x) for x in rev_numbers] + + def getrevisioninfo(self, timeout, *revision_numbers): + if self._mock: + response = self._mocked_responses.pop(0) + if 'error_info' in response: + raise ClientError(response.get('error_code', None), + response['error_info']) + return response['result'] + + rev_numbers = self._normalizerevisionnumbers(revision_numbers) + + ret = self._client.query( + timeout, + ' \ + query RevisionQuery( \ + $params: [DifferentialRevisionQueryParams!]! \ + ) { \ + differential_revision_query(query_params: $params) { \ + results { \ + nodes { \ + number \ + diff_status_name \ + latest_active_diff { \ + local_commit_info: diff_properties ( \ + property_names: ["local:commits"] \ + ) { \ + nodes { \ + property_value \ + } \ + } \ + } \ + differential_diffs { \ + count \ + } \ + } \ + } \ + } \ + } \ + ' + , + {'params': { + 'numbers': rev_numbers, + }}, + ) + revisions = ret['differential_revision_query'][0]['results'][ + 'nodes'] + if revisions is None: + return None + + infos = {} + for revision in rev_numbers: + info = {} + for node in ret['differential_revision_query'][0][ + 'results']['nodes']: + if node['number'] != revision: + continue + + status = node['diff_status_name'] + # GraphQL uses "Closed" but Conduit used "Committed" so let's + # not change the naming + if status == 'Closed': + status = 'Committed' + info['status'] = status + + active_diff = node['latest_active_diff'] + if active_diff is None: + continue + + info['count'] = node['differential_diffs']['count'] + + localcommitnode = active_diff['local_commit_info']['nodes'] + if localcommitnode is not None and len(localcommitnode) == 1: + localcommits = json.loads(localcommitnode[0][ + 'property_value']) + localcommits = sorted(localcommits.values(), + key=itemgetter('time'), reverse=True) + info['hash'] = localcommits[0].get('commit', None) + infos[str(revision)] = info + return infos diff --git a/phabricator/phabricator_graphql_client.py b/phabricator/phabricator_graphql_client.py new file mode 100644 --- /dev/null +++ b/phabricator/phabricator_graphql_client.py @@ -0,0 +1,82 @@ +from __future__ import absolute_import +from __future__ import division +from __future__ import unicode_literals + +import time + +class PhabricatorGraphQLClient(object): + + def __init__(self, + urllib, + ph_cert, + ph_oauth, + ph_user_name, + source, + host, + app_id, + app_token, + ca_bundle=None): + self.urllib = urllib + self.phabricator_certificate = ph_cert + self.phabricator_oauth = ph_oauth + self.user = ph_user_name + self.__cert_time = 0 + self.graphql_url = host + '/graphql' + self.token_url = host + '/phabricator/get_token' + self.source = source + self.app_id = app_id + self.app_token = app_token + self.ca_bundle = ca_bundle + + def query(self, timeout, request, params=None): + """ + Make a graphql2 (OSS) request to phabricator data + """ + self._checkconnection(timeout) + if self.phabricator_oauth is None: + data = {'phabricator_token': self.__cert, 'doc': request, + 'variables': params} + else: + data = {'access_token': self.phabricator_oauth, 'doc': request, + 'variables': params} + + return self.urllib.sendpost( + self.graphql_url, + data=data, + timeout=timeout, + ca_bundle=self.ca_bundle).get('data') + + def _checkconnection(self, timeout): + """ + We only care about the expiring Phabricator token if we don't have + an OAuth token + """ + if self.phabricator_oauth is None: + if time.time() - self.__cert_time > 600: + self._connect(timeout) + data = {'phabricator_token': self.__cert, 'source': self.source} + self.urllib.sendpost( + self.graphql_url, + data=data, + timeout=timeout, + ca_bundle=self.ca_bundle) + + def _connect(self, timeout): + """ + Private method to get token to make calls, unless we're using the + OAuth token, then we just do nothing here + """ + if self.phabricator_oauth is None: + data = { + 'ph_certificate': self.phabricator_certificate, + 'ph_user_name': self.user, + 'app': self.app_id, + 'token': self.app_token + } + res = self.urllib.sendpost( + self.token_url, + data=data, + timeout=timeout, + ca_bundle=self.ca_bundle) + self.__cert = res.get('token') + self.__cert_time = time.time() diff --git a/phabricator/phabricator_graphql_client_requests.py b/phabricator/phabricator_graphql_client_requests.py new file mode 100644 --- /dev/null +++ b/phabricator/phabricator_graphql_client_requests.py @@ -0,0 +1,25 @@ +import requests +import json + +# helper class so phabricator_graphql_client can talk using the requests +# third-party library + +class PhabricatorClientError(Exception): + def __init__(self, reason, error): + Exception.__init__(self, reason, error) + +class PhabricatorGraphQLClientRequests(object): + + def sendpost(self, request_url, data, timeout, ca_bundle): + res = requests.post(request_url, data, verify=ca_bundle or True) + data = json.loads(res.content.decode('utf8')) + if res.status_code != 200: + raise PhabricatorClientError( + 'Phabricator not available ' + 'returned ' + str(res.status), + res) + # Apparently both singular and plural are used. + if 'error' in data: + raise PhabricatorClientError("Error in query", data['error']) + if 'errors' in data: + raise PhabricatorClientError("Error in query", data['errors']) + return data diff --git a/phabricator/phabricator_graphql_client_urllib.py b/phabricator/phabricator_graphql_client_urllib.py new file mode 100644 --- /dev/null +++ b/phabricator/phabricator_graphql_client_urllib.py @@ -0,0 +1,43 @@ +from mercurial.util import httplib +from mercurial import util +import json + +urlreq = util.urlreq + +# helper class so phabricator_graphql_client can talk using the requests +# third-party library + +class PhabricatorClientError(Exception): + def __init__(self, reason, error): + Exception.__init__(self, reason, error) + +class PhabricatorGraphQLClientRequests(object): + + def __init__(self): + self._connection = None + + def __verify_connection(self, request_url, timeout, ca_bundle): + urlparts = urlreq.urlparse(request_url) + if self._connection is None: + if urlparts.scheme == 'http': + self._connection = httplib.HTTPConnection( + urlparts.netloc, timeout=timeout) + elif urlparts.scheme == 'https': + self._connection = httplib.HTTPSConnection( + urlparts.netloc, timeout=timeout, cert_file=ca_bundle) + else: + raise PhabricatorClientError('Unknown host scheme: %s', + urlparts.scheme) + return urlparts + + def sendpost(self, request_url, data, timeout, ca_bundle): + urlparts = self.__verify_connection(request_url, timeout, ca_bundle) + query = util.urlreq.urlencode(data) + headers = { + 'Connection': 'Keep-Alive', + 'Content-Type': 'application/x-www-form-urlencoded', + } + self._connection.request('POST', (urlparts.path), query, headers) + + response = json.load(self._connection.getresponse()) + return response diff --git a/tests/test-check-code-hg.t b/tests/test-check-code-hg.t --- a/tests/test-check-code-hg.t +++ b/tests/test-check-code-hg.t @@ -92,9 +92,6 @@ Skipping ctreemanifest/manifest_ptr.h it has no-che?k-code (glob) Skipping ctreemanifest/treemanifest.cpp it has no-che?k-code (glob) Skipping ctreemanifest/treemanifest.h it has no-che?k-code (glob) - phabricator/conduit.py:16: - > import urllib3 - don't use urllib, use util.urlreq/util.urlerr Skipping tests/conduithttp.py it has no-che?k-code (glob) tests/test-myparent.t:56: > $ hg help templates | grep -A2 myparent diff --git a/tests/test-check-config-hg.t b/tests/test-check-config-hg.t --- a/tests/test-check-config-hg.t +++ b/tests/test-check-config-hg.t @@ -44,6 +44,9 @@ undocumented: perftweaks.disablecasecheck (bool) undocumented: perftweaks.disabletags (bool) undocumented: perftweaks.preferdeltas (bool) + undocumented: phabricator.graphql_app_id (str) + undocumented: phabricator.graphql_app_token (str) + undocumented: phabricator.graphql_host (str) undocumented: phabstatus.logpeekahead (int) [30] undocumented: phrevset.callsign (str) undocumented: pushrebase.blocknonpushrebase (bool) diff --git a/tests/test-check-py3-compat-hg.t b/tests/test-check-py3-compat-hg.t --- a/tests/test-check-py3-compat-hg.t +++ b/tests/test-check-py3-compat-hg.t @@ -75,6 +75,9 @@ linelog/pyext/test-random-edits.py not using absolute_import phabricator/arcconfig.py not using absolute_import phabricator/diffprops.py not using absolute_import + phabricator/graphql.py not using absolute_import + phabricator/phabricator_graphql_client_requests.py not using absolute_import + phabricator/phabricator_graphql_client_urllib.py not using absolute_import remotefilelog/__init__.py not using absolute_import remotefilelog/cacheclient.py not using absolute_import remotefilelog/constants.py not using absolute_import diff --git a/tests/test-diff-since-last-arc-diff.t b/tests/test-diff-since-last-arc-diff.t --- a/tests/test-diff-since-last-arc-diff.t +++ b/tests/test-diff-since-last-arc-diff.t @@ -29,44 +29,34 @@ Prep configuration - $ echo '{}' > .arcconfig $ echo '{}' > .arcrc + $ echo '{"config" : {"default" : "https://a.com/api"}, "hosts" : {"https://a.com/api/" : { "user" : "testuser", "cert" : "garbage_cert"}}}' > .arcconfig Now progressively test the response handling for variations of missing data $ cat > $TESTTMP/mockduit << EOF - > [{"cmd": ["differential.query", {"ids": ["1"]}], "result": null}] + > [{"result": {}}] > EOF $ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg diff --since-last-arc-diff abort: unable to determine previous changeset hash [255] $ cat > $TESTTMP/mockduit << EOF - > [{"cmd": ["differential.query", {"ids": ["1"]}], "result": [{"diffs": []}]}] + > [{"result": {"2" : {}}}] > EOF $ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg diff --since-last-arc-diff abort: unable to determine previous changeset hash [255] $ cat > $TESTTMP/mockduit << EOF - > [{"cmd": ["differential.query", {"ids": ["1"]}], "result": [{"diffs": [1]}]}, - > {"cmd": ["differential.getdiffproperties", { - > "diff_id": 1, - > "names": ["local:commits"]}], - > "result": [] - > }] + > [{"result": {"1" : {"count": 3, "status": "Needs Review"}}}] > EOF $ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg diff --since-last-arc-diff abort: unable to determine previous changeset hash [255] $ cat > $TESTTMP/mockduit << EOF - > [{"cmd": ["differential.query", {"ids": ["1"]}], "result": [{"diffs": [1]}]}, - > {"cmd": ["differential.getdiffproperties", { - > "diff_id": 1, - > "names": ["local:commits"]}], - > "result": {"local:commits": []} - > }] + > [{"result": {"1" : {"status": "Needs Review"}}}] > EOF $ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg diff --since-last-arc-diff abort: unable to determine previous changeset hash @@ -76,15 +66,7 @@ there is no diff since what was landed. $ cat > $TESTTMP/mockduit << EOF - > [{"cmd": ["differential.query", {"ids": ["1"]}], "result": [{"diffs": [1]}]}, - > {"cmd": ["differential.getdiffproperties", { - > "diff_id": 1, - > "names": ["local:commits"]}], - > "result": {"local:commits": { - > "2e6531b7dada2a3e5638e136de05f51e94a427f4": { - > "commit": "2e6531b7dada2a3e5638e136de05f51e94a427f4", - > "time": 0}}} - > }] + > [{"result": {"1" : {"hash": "2e6531b7dada2a3e5638e136de05f51e94a427f4", "count": 1, "status": "Needs Review"}}}] > EOF $ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg diff --since-last-arc-diff @@ -94,18 +76,7 @@ assert that we order the commits consistently based on the time field. $ cat > $TESTTMP/mockduit << EOF - > [{"cmd": ["differential.query", {"ids": ["1"]}], "result": [{"diffs": [1]}]}, - > {"cmd": ["differential.getdiffproperties", { - > "diff_id": 1, - > "names": ["local:commits"]}], - > "result": {"local:commits": { - > "88dd5a13bf28b99853a24bddfc93d4c44e07c6bd": { - > "commit": "88dd5a13bf28b99853a24bddfc93d4c44e07c6bd", - > "time": 10}, - > "fakehashtotestsorting": { - > "commit": "fakehashtotestsorting", - > "time": 5}}} - > }] + > [{"result": {"1" : {"hash": "88dd5a13bf28b99853a24bddfc93d4c44e07c6bd", "count": 1, "status": "Needs Review"}}}] > EOF $ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg diff --since-last-arc-diff --nodates diff -r 88dd5a13bf28 foo diff --git a/tests/test-fastlog.t b/tests/test-fastlog.t --- a/tests/test-fastlog.t +++ b/tests/test-fastlog.t @@ -2,7 +2,6 @@ > [extensions] > tweakdefaults=$TESTDIR/../hgext3rd/tweakdefaults.py > fastlog=$TESTDIR/../hgext3rd/fastlog.py - > fbconduit=$TESTDIR/../hgext3rd/fbconduit.py > [fbconduit] > host=our.intern.facebook.com > protocol=http @@ -63,6 +62,7 @@ o 0 a + Create a merge $ hg merge --config tweakdefaults.allowmerge=True @@ -249,22 +249,3 @@ 11c9870ffc4024fab11bf166a00b2852ea36bcf6 5946a2427fdfcb068a8aec1a59227d0d76062b43 728676e01661ccc3d7e39de054ca3a7288d7e7b6 - -Test with failed network call - $ cat > $TESTTMP/bad_conduit.py <<'EOF' - > def call_conduit(*args, **kwargs): - > raise Exception('failed') - > def conduit_config(*args, **kwargs): - > return True - > EOF - - $ cat >> $HGRCPATH << EOF - > [extensions] - > fbconduit=$TESTTMP/bad_conduit.py - > EOF - - $ hg log parent -T '{rev} {desc}\n' 2>&1 | grep --invert-match 'failed' - 9 toys - 8 treats - 7 cookies - 6 major repo reorg diff --git a/tests/test-phabstatus.t b/tests/test-phabstatus.t --- a/tests/test-phabstatus.t +++ b/tests/test-phabstatus.t @@ -22,8 +22,8 @@ Configure arc... - $ echo '{}' > .arcconfig $ echo '{}' > .arcrc + $ echo '{"config" : {"default" : "https://a.com/api"}, "hosts" : {"https://a.com/api/" : { "user" : "testuser", "cert" : "garbage_cert"}}}' > .arcconfig And now with bad responses: @@ -78,7 +78,9 @@ information. We intentionally do not use HG_ARC_CONDUIT_MOCK for this test, so it tries to parse the (empty) arc config files. + $ echo '{}' > .arcrc + $ echo '{}' > .arcconfig $ hg log -T '{phabstatus}\n' -r . arcconfig configuration problem. No diff information can be provided. - Error info: arcrc is missing user credentials for host https://phabricator.intern.facebook.com/api/. use "arc install-certificate" to fix. + Error info: arcrc is missing user credentials for host None. use "arc install-certificate" to fix. Error diff --git a/tests/test-syncstatus.t b/tests/test-syncstatus.t --- a/tests/test-syncstatus.t +++ b/tests/test-syncstatus.t @@ -22,15 +22,15 @@ Configure arc... - $ echo '{}' > .arcconfig $ echo '{}' > .arcrc + $ echo '{"config" : {"default" : "https://a.com/api"}, "hosts" : {"https://a.com/api/" : { "user" : "testuser", "cert" : "garbage_cert"}}}' > .arcconfig And now with bad responses: $ cat > $TESTTMP/mockduit << EOF > [{"cmd": ["differential.querydiffhashes", {"revisionIDs": ["1"]}], "result": {}}] > EOF - $ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{syncstatus}\n' -r . + $ OVERRIDE_GRAPHQL_URI=https://a.com HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{syncstatus}\n' -r . Error $ cat > $TESTTMP/mockduit << EOF @@ -76,4 +76,3 @@ > EOF $ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{syncstatus}\n' -r . committed -