From 75f29f9315a04a540222aba4f2440dca13607080 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 8 Apr 2024 13:23:56 +0200 Subject: [PATCH] [IMP] conftest: avoid parsing json it if may not be When asserting a status fails, it's possible that this is a 400 or 500-type error which does not yield JSON data at all (e.g. forgot to start the proxy or dummy), in which case trying to dump the json data is actively harmful as it triggers cascading failures. And it's not like the output message is formatted, so a re-structured assertion message is not helpful. --- conftest.py | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/conftest.py b/conftest.py index 090ebb4b..2bf6f4e0 100644 --- a/conftest.py +++ b/conftest.py @@ -585,14 +585,13 @@ class Repo: assert self.hook r = self._session.get( 'https://api.github.com/repos/{}/hooks'.format(self.name)) - response = r.json() - assert 200 <= r.status_code < 300, response - [hook] = response + assert 200 <= r.status_code < 300, r.text + [hook] = r.json() r = self._session.patch('https://api.github.com/repos/{}/hooks/{}'.format(self.name, hook['id']), json={ 'config': {**hook['config'], 'secret': secret}, }) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text def get_ref(self, ref): # differs from .commit(ref).id for the sake of assertion error messages @@ -628,7 +627,7 @@ class Repo: ref = 'refs/' + ref r = self._session.get('https://api.github.com/repos/{}/commits/{}'.format(self.name, ref)) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text return self._commit_from_gh(r.json()) @@ -650,14 +649,14 @@ class Repo: :rtype: Dict[str, str] """ r = self._session.get('https://api.github.com/repos/{}/git/trees/{}'.format(self.name, commit.tree)) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text # read tree's blobs tree = {} for t in r.json()['tree']: assert t['type'] == 'blob', "we're *not* doing recursive trees in test cases" r = self._session.get('https://api.github.com/repos/{}/git/blobs/{}'.format(self.name, t['sha'])) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text tree[t['path']] = base64.b64decode(r.json()['content']).decode() return tree @@ -687,7 +686,7 @@ class Repo: 'required_pull_request_reviews': None, 'restrictions': None, }) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text # FIXME: remove this (runbot_merge should use make_commits directly) def make_commit(self, ref, message, author, committer=None, tree=None, wait=True): @@ -832,7 +831,7 @@ class Repo: }, headers=headers, ) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text return PR(self, r.json()['number']) @@ -845,7 +844,7 @@ class Repo: 'context': context, **kw }) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text def is_ancestor(self, sha, of): return any(c['sha'] == sha for c in self.log(of)) @@ -856,7 +855,7 @@ class Repo: 'https://api.github.com/repos/{}/commits'.format(self.name), params={'sha': ref_or_sha, 'page': page} ) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text yield from r.json() if not r.links.get('next'): return @@ -924,7 +923,7 @@ class PR: 'https://api.github.com/repos/{}/pulls/{}'.format(self.repo.name, self.number), headers=caching ) - assert r.ok, r.json() + assert r.ok, r.text if r.status_code == 304: return previous contents, caching = self._cache = r.json(), {} @@ -969,7 +968,7 @@ class PR: @property def comments(self): r = self.repo._session.get('https://api.github.com/repos/{}/issues/{}/comments'.format(self.repo.name, self.number)) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text return [Comment(c) for c in r.json()] @property @@ -986,7 +985,7 @@ class PR: json={'body': body}, headers=headers, ) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text return r.json()['id'] def edit_comment(self, cid, body, token=None): @@ -999,7 +998,7 @@ class PR: json={'body': body}, headers=headers ) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text wait_for_hook() def delete_comment(self, cid, token=None): @@ -1011,7 +1010,7 @@ class PR: 'https://api.github.com/repos/{}/issues/comments/{}'.format(self.repo.name, cid), headers=headers ) - assert r.status_code == 204, r.json() + assert r.status_code == 204, r.text def _set_prop(self, prop, value, token=None): assert self.repo.hook @@ -1035,7 +1034,7 @@ class PR: self.repo.name, self.number, )) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text info = r.json() repo = self.repo @@ -1056,7 +1055,7 @@ class PR: json={'body': body, 'event': state,}, headers=headers ) - assert 200 <= r.status_code < 300, r.json() + assert 200 <= r.status_code < 300, r.text PRBranch = collections.namedtuple('PRBranch', 'repo branch') class LabelsProxy(collections.abc.MutableSet): @@ -1067,7 +1066,7 @@ class LabelsProxy(collections.abc.MutableSet): def _labels(self): pr = self._pr r = pr.repo._session.get('https://api.github.com/repos/{}/issues/{}/labels'.format(pr.repo.name, pr.number)) - assert r.ok, r.json() + assert r.ok, r.text return {label['name'] for label in r.json()} def __repr__(self): @@ -1093,14 +1092,14 @@ class LabelsProxy(collections.abc.MutableSet): r = pr.repo._session.post('https://api.github.com/repos/{}/issues/{}/labels'.format(pr.repo.name, pr.number), json={ 'labels': [label] }) - assert r.ok, r.json() + assert r.ok, r.text def discard(self, label): pr = self._pr assert pr.repo.hook r = pr.repo._session.delete('https://api.github.com/repos/{}/issues/{}/labels/{}'.format(pr.repo.name, pr.number, label)) # discard should do nothing if the item didn't exist in the set - assert r.ok or r.status_code == 404, r.json() + assert r.ok or r.status_code == 404, r.text def update(self, *others): pr = self._pr @@ -1109,7 +1108,7 @@ class LabelsProxy(collections.abc.MutableSet): r = pr.repo._session.post('https://api.github.com/repos/{}/issues/{}/labels'.format(pr.repo.name, pr.number), json={ 'labels': list(set(itertools.chain.from_iterable(others))) }) - assert r.ok, r.json() + assert r.ok, r.text class Environment: def __init__(self, port, db, default_crons=()):