[reprotest] 01/07: main: factor common args into a namedtuple to avoid passing them individually

Ximin Luo infinity0 at debian.org
Tue Sep 26 14:36:08 UTC 2017


This is an automated email from the git hooks/post-receive script.

infinity0 pushed a commit to branch master
in repository reprotest.

commit 4e05689e872d504ce88f122b50964721f10da5a8
Author: Ximin Luo <infinity0 at debian.org>
Date:   Tue Sep 26 14:14:18 2017 +0200

    main: factor common args into a namedtuple to avoid passing them individually
---
 reprotest/__init__.py   | 96 +++++++++++++++++++++++++++----------------------
 tests/test_reprotest.py | 46 ++++++++++++------------
 2 files changed, 77 insertions(+), 65 deletions(-)

diff --git a/reprotest/__init__.py b/reprotest/__init__.py
index 27d31e2..cfd5623 100644
--- a/reprotest/__init__.py
+++ b/reprotest/__init__.py
@@ -91,7 +91,22 @@ def coroutine(func):
     return start
 
 
-class BuildContext(collections.namedtuple('_BuildContext', 'testbed_root local_dist_root local_src build_name variations')):
+ at contextlib.contextmanager
+def empty_or_temp_dir(empty_dir, name):
+    if empty_dir:
+        empty_dir = str(empty_dir)
+        if not os.path.exists(empty_dir):
+            os.makedirs(empty_dir, exist_ok=False)
+        elif os.listdir(empty_dir):
+            raise ValueError("%s must be empty: %s" % (name, empty_dir))
+        yield empty_dir
+    else:
+        with tempfile.TemporaryDirectory() as temp_dir:
+            yield temp_dir
+
+
+class BuildContext(collections.namedtuple('_BuildContext',
+    'testbed_root local_dist_root local_src build_name variations')):
     """
 
     The idiom os.path.join(x, '') is used here to ensure a trailing directory
@@ -187,16 +202,34 @@ def run_diff(dist_0, dist_1, diffoscope_args, store_dir):
     return retcode
 
 
+class TestbedArgs(collections.namedtuple('_TestbedArgs',
+    'virtual_server_args testbed_pre testbed_init host_distro')):
+    @classmethod
+    def default(cls, virtual_server_args=[], testbed_pre=None, testbed_init=None, host_distro='debian'):
+        return cls(virtual_server_args, testbed_pre, testbed_init, host_distro)
+
+
+class TestArgs(collections.namedtuple('_Test',
+    'build_command source_root artifact_pattern result_dir no_clean_on_error diffoscope_args')):
+    @classmethod
+    def default(cls, build_command, source_root, artifact_pattern, result_dir=None,
+                no_clean_on_error=False, diffoscope_args=[]):
+        return cls(build_command, source_root, artifact_pattern, result_dir,
+                   no_clean_on_error, diffoscope_args)
+
+
 @coroutine
-def corun_builds(build_command, source_root, artifact_pattern, result_dir, no_clean_on_error,
-                 virtual_server_args, testbed_pre, testbed_init, host_distro):
+def corun_builds(test_args, testbed_args):
     """A coroutine for running the builds.
 
-    .>>> proc = corun_builds(...)
+    .>>> proc = corun_builds(test_args, testbed_args)
     .>>> for name, var in variations:
     .>>>     local_dist = proc.send((name, var))
     .>>>     ...
     """
+    build_command, source_root, artifact_pattern, result_dir, no_clean_on_error, diffoscope_args = test_args
+    virtual_server_args, testbed_pre, testbed_init, host_distro = testbed_args
+
     if not source_root:
         raise ValueError("invalid source root: %s" % source_root)
     if os.path.isfile(source_root):
@@ -240,7 +273,7 @@ def corun_builds(build_command, source_root, artifact_pattern, result_dir, no_cl
                 logging.log(5, "build %s: %r", name, build)
 
                 if testbed_init:
-                    testbed.check_exec(["sh", "-ec", testbed_init])
+                    check_exec(["sh", "-ec", testbed_init])
 
                 bctx.copydown(testbed)
                 bctx.run_build(testbed, build, artifact_pattern)
@@ -249,29 +282,12 @@ def corun_builds(build_command, source_root, artifact_pattern, result_dir, no_cl
                 name_variation = yield bctx.local_dist
 
 
- at contextlib.contextmanager
-def empty_or_temp_dir(empty_dir, name):
-    if empty_dir:
-        empty_dir = str(empty_dir)
-        if not os.path.exists(empty_dir):
-            os.makedirs(empty_dir, exist_ok=False)
-        elif os.listdir(empty_dir):
-            raise ValueError("%s must be empty: %s" % (name, empty_dir))
-        yield empty_dir
-    else:
-        with tempfile.TemporaryDirectory() as temp_dir:
-            yield temp_dir
-
-
-def check(build_command, source_root, artifact_pattern, store_dir=None, no_clean_on_error=False,
-          virtual_server_args=[], testbed_pre=None, testbed_init=None, host_distro='debian',
-          build_variations=Variations.of(VariationSpec.default()), diffoscope_args=[]):
+def check(test_args, testbed_args, build_variations=Variations.of(VariationSpec.default())):
     # default argument [] is safe here because we never mutate it.
+    _, _, artifact_pattern, store_dir, _, diffoscope_args = test_args
     with empty_or_temp_dir(store_dir, "store_dir") as result_dir:
         assert store_dir == result_dir or store_dir is None
-        proc = corun_builds(
-            build_command, source_root, artifact_pattern, result_dir, no_clean_on_error,
-            virtual_server_args, testbed_pre, testbed_init, host_distro)
+        proc = corun_builds(test_args._replace(result_dir=result_dir), testbed_args)
 
         bnames = ["control"] + ["experiment-%s" % i for i in range(1, len(build_variations))]
         local_dists = [proc.send(nv) for nv in zip(bnames, build_variations)]
@@ -303,15 +319,12 @@ def check(build_command, source_root, artifact_pattern, store_dir=None, no_clean
         return not retcode
 
 
-def check_auto(build_command, source_root, artifact_pattern, store_dir=None, no_clean_on_error=False,
-          virtual_server_args=[], testbed_pre=None, testbed_init=None, host_distro='debian',
-          build_variations=Variations.of(VariationSpec.default()), diffoscope_args=[]):
+def check_auto(test_args, testbed_args, build_variations=Variations.of(VariationSpec.default())):
     # default argument [] is safe here because we never mutate it.
+    _, _, _, store_dir, _, diffoscope_args = test_args
     with empty_or_temp_dir(store_dir, "store_dir") as result_dir:
         assert store_dir == result_dir or store_dir is None
-        proc = corun_builds(
-            build_command, source_root, artifact_pattern, result_dir, no_clean_on_error,
-            virtual_server_args, testbed_pre, testbed_init, host_distro)
+        proc = corun_builds(test_args._replace(result_dir=result_dir), testbed_args)
 
         var_x0, var_x1 = build_variations
         dist_x0 = proc.send(("control", var_x0))
@@ -436,8 +449,8 @@ def cli_parser():
     group1.add_argument('-s', '--source-root', default=None,
         help='Root of the source tree, that is copied to the virtual server '
         'and made available during the build. If a file is given here, then '
-        'all files in its parent directory are available during the build. '
-        'Default: "." (current working directory).')
+        'its parent directory is used instead. Default: "." (current working '
+        'directory).')
     group1.add_argument('-c', '--build-command', default=None,
         help='Build command to execute. If this is "auto" then reprotest will '
         'guess how to build the given source_root, in which case various other '
@@ -656,17 +669,16 @@ def run(argv, dry_run=None):
         print("No <artifact> to test for differences provided. See --help for options.")
         sys.exit(2)
 
-    check_args_keys = (
-        "build_command", "artifact_pattern", "virtual_server_args", "source_root",
-        "no_clean_on_error", "store_dir", "diffoscope_args", "build_variations",
-        "testbed_pre", "testbed_init", "host_distro")
-    l = locals()
-    check_args = collections.OrderedDict([(k, l[k]) for k in check_args_keys])
+    testbed_args = TestbedArgs(virtual_server_args, testbed_pre, testbed_init, host_distro)
+    test_args = TestArgs(build_command, source_root, artifact_pattern, store_dir,
+                         no_clean_on_error, diffoscope_args)
+
+    check_args = (test_args, testbed_args, build_variations)
     if parsed_args.dry_run or dry_run:
         return check_args
     else:
         try:
-            return 0 if check_func(**check_args) else 1
+            return 0 if check_func(*check_args) else 1
         except Exception:
             traceback.print_exc()
             return 125
@@ -674,8 +686,8 @@ def run(argv, dry_run=None):
 
 def main():
     r = run(sys.argv[1:])
-    if isinstance(r, collections.OrderedDict):
+    if not isinstance(r, int):
         import pprint
-        pprint.pprint(r)
+        pprint.pprint(r, width=40, compact=True)
     else:
         return r
diff --git a/tests/test_reprotest.py b/tests/test_reprotest.py
index f1d7f72..99875a7 100644
--- a/tests/test_reprotest.py
+++ b/tests/test_reprotest.py
@@ -7,7 +7,7 @@ import sys
 
 import pytest
 import reprotest
-import reprotest.build
+from reprotest.build import VariationSpec, Variations, VARIATIONS
 
 REPROTEST = [sys.executable, "-m", "reprotest", "--no-diffoscope"]
 REPROTEST_TEST_SERVERS = os.getenv("REPROTEST_TEST_SERVERS", "null").split(",")
@@ -16,13 +16,13 @@ REPROTEST_TEST_DONTVARY = os.getenv("REPROTEST_TEST_DONTVARY", "").split(",")
 if REPROTEST_TEST_DONTVARY:
     REPROTEST += ["--vary=-" + ",-".join(REPROTEST_TEST_DONTVARY)]
 
-TEST_VARIATIONS = frozenset(reprotest.build.VARIATIONS.keys()) - frozenset(REPROTEST_TEST_DONTVARY)
+TEST_VARIATIONS = frozenset(VARIATIONS.keys()) - frozenset(REPROTEST_TEST_DONTVARY)
 
 def check_reproducibility(command, virtual_server, reproducible):
-    build_variations = reprotest.build.Variations.of(
-        reprotest.build.VariationSpec.default(TEST_VARIATIONS))
-    result = reprotest.check(command, 'tests', 'artifact',
-        virtual_server_args=virtual_server, build_variations=build_variations)
+    result = reprotest.check(
+        reprotest.TestArgs.default(command, 'tests', 'artifact'),
+        reprotest.TestbedArgs.default(virtual_server),
+        Variations.of(VariationSpec.default(TEST_VARIATIONS)))
     assert result == reproducible
 
 def check_command_line(command_line, code=None):
@@ -57,7 +57,7 @@ def test_simple_builds(virtual_server):
     check_reproducibility('python3 mock_build.py irreproducible', virtual_server, False)
 
 # TODO: test all variations that we support
- at pytest.mark.parametrize('captures', list(reprotest.build.VARIATIONS.keys()))
+ at pytest.mark.parametrize('captures', list(VARIATIONS.keys()))
 def test_variations(virtual_server, captures):
     expected = captures not in TEST_VARIATIONS
     check_reproducibility('python3 mock_build.py ' + captures, virtual_server, expected)
@@ -72,34 +72,34 @@ def test_self_build(virtual_server):
     assert(1 == subprocess.call(REPROTEST + ['python3 setup.py bdist_wheel', 'dist/*.whl'] + virtual_server))
 
 def test_command_lines():
-    r = check_command_line(".".split(), 0)
-    assert r['artifact_pattern'] is not None
-    r = check_command_line(". -- null -d".split(), 0)
-    assert r['artifact_pattern'] is not None
+    test_args, _, _ = check_command_line(".".split(), 0)
+    assert test_args.artifact_pattern is not None
+    test_args, _, _ = check_command_line(". -- null -d".split(), 0)
+    assert test_args.artifact_pattern is not None
     check_command_line("--dry-run . --verbosity 2 -- null -d".split(), 0)
-    assert r['artifact_pattern'] is not None
+    assert test_args.artifact_pattern is not None
     check_command_line(". null -d".split(), 2)
     check_command_line(". --verbosity 2 null -d".split(), 2)
     check_command_line("--dry-run . --verbosity 2 null -d".split(), 2)
     check_command_line("--dry-run . null -d".split(), 2)
 
-    r = check_command_line("auto".split(), 0)
-    assert r['artifact_pattern'] is not None
-    r = check_command_line("auto -- null -d".split(), 0)
-    assert r['artifact_pattern'] is not None
+    test_args, _, _ = check_command_line("auto".split(), 0)
+    assert test_args.artifact_pattern is not None
+    test_args, _, _ = check_command_line("auto -- null -d".split(), 0)
+    assert test_args.artifact_pattern is not None
     check_command_line("--dry-run auto --verbosity 2 -- null -d".split(), 0)
-    assert r['artifact_pattern'] is not None
+    assert test_args.artifact_pattern is not None
     check_command_line("auto null -d".split(), 2)
     check_command_line("auto --verbosity 2 null -d".split(), 2)
     check_command_line("--dry-run auto --verbosity 2 null -d".split(), 2)
     check_command_line("--dry-run auto null -d".split(), 2)
 
-    r = check_command_line("auto -- schroot unstable-amd64-sbuild".split(), 0)
-    assert r['virtual_server_args'] == ['schroot', 'unstable-amd64-sbuild']
-    r = check_command_line(". -- schroot unstable-amd64-sbuild".split(), 0)
-    assert r['virtual_server_args'] == ['schroot', 'unstable-amd64-sbuild']
-    r = check_command_line("auto . schroot unstable-amd64-sbuild".split(), 0)
-    assert r['virtual_server_args'] == ['schroot', 'unstable-amd64-sbuild']
+    _, testbed_args, _ = check_command_line("auto -- schroot unstable-amd64-sbuild".split(), 0)
+    assert testbed_args.virtual_server_args == ['schroot', 'unstable-amd64-sbuild']
+    _, testbed_args, _ = check_command_line(". -- schroot unstable-amd64-sbuild".split(), 0)
+    assert testbed_args.virtual_server_args == ['schroot', 'unstable-amd64-sbuild']
+    _, testbed_args, _ = check_command_line("auto . schroot unstable-amd64-sbuild".split(), 0)
+    assert testbed_args.virtual_server_args == ['schroot', 'unstable-amd64-sbuild']
 
 # TODO: don't call it if we don't have debian/, e.g. for other distros
 def test_debian_build(virtual_server):

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/reprotest.git



More information about the Reproducible-commits mailing list