From cd200f6d34fb2dfff7900d50c058d337e5c701d9 Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Tue, 26 Jun 2018 00:13:58 +0000 Subject: [PATCH] Bug 1413922 - [mozversioncontrol] Always use hglib.client if available and fall back to subprocesses if not, r=gps Most HG commands use subprocesses, even if a context manager (and therefore an hglib client) has been created. There are only two commands that make use of the client, but they *only* work inside a context manager. I don't think there are any technical reason these two commands *need* to use the context manager. This patch merges the HgRepository._run_in_client function with HgRepository._run(). If a client exists, that will be used, otherwise a subprocess will be used. Differential Revision: https://phabricator.services.mozilla.com/D1809 --HG-- extra : moz-landing-system : lando --- .../mozversioncontrol/__init__.py | 10 +-- python/mozversioncontrol/test/conftest.py | 74 +++++++++++++++++++ python/mozversioncontrol/test/python.ini | 1 + .../test/test_context_manager.py | 30 ++++++++ .../test/test_workdir_outgoing.py | 60 +-------------- 5 files changed, 112 insertions(+), 63 deletions(-) create mode 100644 python/mozversioncontrol/test/conftest.py create mode 100644 python/mozversioncontrol/test/test_context_manager.py diff --git a/python/mozversioncontrol/mozversioncontrol/__init__.py b/python/mozversioncontrol/mozversioncontrol/__init__.py index 2dbc04995ac5..561a0855eb71 100644 --- a/python/mozversioncontrol/mozversioncontrol/__init__.py +++ b/python/mozversioncontrol/mozversioncontrol/__init__.py @@ -236,10 +236,9 @@ class HgRepository(Repository): def __exit__(self, exc_type, exc_val, exc_tb): self._client.close() - def _run_in_client(self, args): + def _run(self, *args, **runargs): if not self._client.server: - raise Exception('active HgRepository context manager required') - + return super(HgRepository, self)._run(*args, **runargs) return self._client.rawcommand(args) def sparse_checkout_present(self): @@ -304,8 +303,7 @@ class HgRepository(Repository): def get_files_in_working_directory(self): # Can return backslashes on Windows. Normalize to forward slashes. return list(p.replace('\\', '/') for p in - self._run_in_client([b'files', b'-0']).split(b'\0') - if p) + self._run(b'files', b'-0').split(b'\0') if p) def working_directory_clean(self, untracked=False, ignored=False): args = [b'status', b'\0', b'--modified', b'--added', b'--removed', @@ -317,7 +315,7 @@ class HgRepository(Repository): # If output is empty, there are no entries of requested status, which # means we are clean. - return not len(self._run_in_client(args).strip()) + return not len(self._run(*args).strip()) class GitRepository(Repository): diff --git a/python/mozversioncontrol/test/conftest.py b/python/mozversioncontrol/test/conftest.py new file mode 100644 index 000000000000..6dc9f8a89eb4 --- /dev/null +++ b/python/mozversioncontrol/test/conftest.py @@ -0,0 +1,74 @@ +# 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/. + +from __future__ import absolute_import + +import os +import subprocess + +import pytest + + +SETUP = { + 'hg': [ + """ + echo "foo" > foo + echo "bar" > bar + hg init + hg add * + hg commit -m "Initial commit" + """, + """ + echo "[paths]\ndefault = ../remoterepo" > .hg/hgrc + """, + ], + 'git': [ + """ + echo "foo" > foo + echo "bar" > bar + git init + git add * + git commit -am "Initial commit" + """, + """ + git remote add upstream ../remoterepo + git fetch upstream + git branch -u upstream/master + """, + ] +} + + +def shell(cmd): + subprocess.check_call(cmd, shell=True) + + +@pytest.yield_fixture(params=['git', 'hg']) +def repo(tmpdir, request): + vcs = request.param + steps = SETUP[vcs] + + if hasattr(request.module, 'STEPS'): + steps.extend(request.module.STEPS[vcs]) + + # tmpdir and repo are py.path objects + # http://py.readthedocs.io/en/latest/path.html + repo = tmpdir.mkdir('repo') + repo.vcs = vcs + + # This creates a step iterator. Each time next() is called + # on it, the next set of instructions will be executed. + repo.step = (shell(cmd) for cmd in steps) + + oldcwd = os.getcwd() + os.chdir(repo.strpath) + + next(repo.step) + + repo.copy(tmpdir.join('remoterepo')) + + next(repo.step) + + yield repo + os.chdir(oldcwd) diff --git a/python/mozversioncontrol/test/python.ini b/python/mozversioncontrol/test/python.ini index 2f38e39f51ab..0dd169a4c08f 100644 --- a/python/mozversioncontrol/test/python.ini +++ b/python/mozversioncontrol/test/python.ini @@ -2,4 +2,5 @@ subsuite=mozversioncontrol skip-if = python == 3 +[test_context_manager.py] [test_workdir_outgoing.py] diff --git a/python/mozversioncontrol/test/test_context_manager.py b/python/mozversioncontrol/test/test_context_manager.py new file mode 100644 index 000000000000..8f7f6a205373 --- /dev/null +++ b/python/mozversioncontrol/test/test_context_manager.py @@ -0,0 +1,30 @@ +# 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/. + +from __future__ import absolute_import + +import mozunit + +from mozversioncontrol import get_repository_object + + +def test_context_manager(repo): + is_git = repo.vcs == 'git' + cmd = ['show', '--no-patch'] if is_git else ['tip'] + + vcs = get_repository_object(repo.strpath) + output_subprocess = vcs._run(*cmd) + assert is_git or vcs._client.server is None + assert "Initial commit" in output_subprocess + + with vcs: + assert is_git or vcs._client.server is not None + output_client = vcs._run(*cmd) + + assert is_git or vcs._client.server is None + assert output_subprocess == output_client + + +if __name__ == '__main__': + mozunit.main() diff --git a/python/mozversioncontrol/test/test_workdir_outgoing.py b/python/mozversioncontrol/test/test_workdir_outgoing.py index 3f6bbd7c9da4..bb2cf84b6952 100644 --- a/python/mozversioncontrol/test/test_workdir_outgoing.py +++ b/python/mozversioncontrol/test/test_workdir_outgoing.py @@ -5,26 +5,14 @@ from __future__ import absolute_import import os -import subprocess import mozunit -import pytest from mozversioncontrol import get_repository_object -setup = { +STEPS = { 'hg': [ - """ - echo "foo" > foo - echo "bar" > bar - hg init - hg add * - hg commit -m "Initial commit" - """, - """ - echo "[paths]\ndefault = ../remoterepo" > .hg/hgrc - """, """ echo "bar" >> bar echo "baz" > baz @@ -36,18 +24,6 @@ setup = { """, ], 'git': [ - """ - echo "foo" > foo - echo "bar" > bar - git init - git add * - git commit -am "Initial commit" - """, - """ - git remote add upstream ../remoterepo - git fetch upstream - git branch -u upstream/master - """, """ echo "bar" >> bar echo "baz" > baz @@ -61,36 +37,6 @@ setup = { } -def shell(cmd): - subprocess.check_call(cmd, shell=True) - - -@pytest.yield_fixture(params=['git', 'hg']) -def repo(tmpdir, request): - vcs = request.param - - # tmpdir and repo are py.path objects - # http://py.readthedocs.io/en/latest/path.html - repo = tmpdir.mkdir('repo') - repo.vcs = vcs - - # This creates a setup iterator. Each time next() is called - # on it, the next set of instructions will be executed. - repo.setup = (shell(cmd) for cmd in setup[vcs]) - - oldcwd = os.getcwd() - os.chdir(repo.strpath) - - next(repo.setup) - - repo.copy(tmpdir.join('remoterepo')) - - next(repo.setup) - - yield repo - os.chdir(oldcwd) - - def assert_files(actual, expected): assert set(map(os.path.basename, actual)) == set(expected) @@ -101,7 +47,7 @@ def test_workdir_outgoing(repo): remotepath = '../remoterepo' if repo.vcs == 'hg' else 'upstream/master' - next(repo.setup) + next(repo.step) assert_files(vcs.get_changed_files('AM', 'all'), ['bar', 'baz']) if repo.vcs == 'git': @@ -111,7 +57,7 @@ def test_workdir_outgoing(repo): assert_files(vcs.get_outgoing_files('AM'), []) assert_files(vcs.get_outgoing_files('AM', remotepath), []) - next(repo.setup) + next(repo.step) assert_files(vcs.get_changed_files('AM', 'all'), []) assert_files(vcs.get_changed_files('AM', 'staged'), [])