Improve test performance and organization (ansible#318)

Improve test performance and organization

Move unit tests to test/unit.
Use tmp_path fixture instead of tmpdir
Use mocker fixture

Reviewed-by: David Shrewsbury <None>
Reviewed-by: None <None>

ansible_builder/utils.py

@@ -155,7 +155,7 @@ def copy_file(source: str, dest: str) -> bool:
         should_copy = True
 
     if should_copy:
-        shutil.copy(source, dest)
+        shutil.copy2(source, dest)
     else:
         logger.debug("File {0} is already up-to-date.".format(dest))
 

test/conftest.py

@@ -1,40 +1,41 @@
-import yaml
-from unittest import mock
+import pathlib
+
 import pytest
-import os
+import yaml
 
 
 @pytest.fixture(autouse=True)
-def do_not_run_commands(request):
+def do_not_run_commands(request, mocker):
     if 'run_command' in request.keywords:
         yield
         return
-    cmd_mock = mock.MagicMock(return_value=[1, [
+    cmd_mock = mocker.MagicMock(return_value=[1, [
         'python:', '  foo: []', 'system: {}',
     ]])
-    with mock.patch('ansible_builder.main.run_command', new=cmd_mock):
-        yield cmd_mock
+    mocker.patch('ansible_builder.main.run_command', new=cmd_mock)
+    yield cmd_mock
 
 
 @pytest.fixture(scope='session')
 def data_dir():
-    return os.path.abspath(os.path.join(os.path.dirname(__file__), 'data'))
+    return pathlib.Path(pathlib.Path(__file__).parent).joinpath('data')
 
 
 @pytest.fixture
-def exec_env_definition_file(tmpdir):
+def exec_env_definition_file(tmp_path):
 
     def _write_file(content=None):
-        path = tmpdir.mkdir('aee').join('execution-env.yml')
+        path = tmp_path / 'aee'
+        path.mkdir()
+        path = path / 'execution-env.yml'
 
         write_str = {}
         if isinstance(content, dict):
             write_str = yaml.dump(content)
         elif isinstance(content, str):
             write_str = content
 
-        with open(path, 'w') as outfile:
-            outfile.write(write_str)
+        path.write_text(write_str)
 
         return path
 
@@ -45,22 +46,26 @@ def _write_file(content=None):
 
 
 @pytest.fixture
-def good_exec_env_definition_path(tmpdir):
-    path = tmpdir.mkdir('aee').join('execution-env.yml')
+def good_exec_env_definition_path(tmp_path):
+    path = tmp_path / 'aee'
+    path.mkdir()
+    path = path / 'execution-env.yml'
 
-    with open(path, 'w') as outfile:
+    with path.open('w') as outfile:
         yaml.dump(good_content, outfile)
 
-    return str(path)
+    return path
 
 
 @pytest.fixture
-def galaxy_requirements_file(tmpdir):
+def galaxy_requirements_file(tmp_path):
 
     def _write_file(content={}):
-        path = tmpdir.mkdir('galaxy').join('requirements.yml')
+        path = tmp_path / 'galaxy'
+        path.mkdir()
+        path = path / 'requirements.yml'
 
-        with open(path, 'w') as outfile:
+        with path.open('w') as outfile:
             yaml.dump(content, outfile)
 
         return path

test/integration/conftest.py

@@ -1,11 +1,10 @@
+import logging
 import os
+import re
 import subprocess
-
-import tempfile
 import uuid
 
 import pytest
-import logging
 
 logger = logging.getLogger(__name__)
 
@@ -14,14 +13,14 @@
 
 
 @pytest.fixture
-def build_dir_and_ee_yml():
+def build_dir_and_ee_yml(tmp_path):
     """Fixture to return temporary file maker."""
 
     def tmp_dir_and_file(ee_contents):
-        tmpdir = tempfile.mkdtemp(prefix="ansible-builder-test-")
-        with tempfile.NamedTemporaryFile(delete=False, dir=tmpdir) as tempf:
-            tempf.write(bytes(ee_contents, "UTF-8"))
-        return tmpdir, tempf.name
+        tmp_file = tmp_path / 'ee.txt'
+        tmp_file.write_text(ee_contents)
+
+        return tmp_path, tmp_file
 
     return tmp_dir_and_file
 
@@ -78,9 +77,10 @@ def delete_image(container_runtime, image_name):
         return
     # delete given image, if the test happened to make one
     # allow error in case that image was not created
+    regexp = re.compile(r'(no such image)|(image not known)|(image is in use by a container)', re.IGNORECASE)
     r = run(f'{container_runtime} rmi -f {image_name}', allow_error=True)
     if r.rc != 0:
-        if 'no such image' in r.stdout or 'no such image' in r.stderr or 'image not known' in r.stdout or 'image not known' in r.stderr:
+        if regexp.search(r.stdout) or regexp.search(r.stderr):
             return
         else:
             raise Exception(f'Teardown failed (rc={r.rc}):\n{r.stdout}\n{r.stderr}')

test/integration/test_build.py

@@ -2,13 +2,13 @@
 import os
 
 
-def test_build_fail_exitcode(cli, container_runtime, ee_tag, tmpdir, data_dir):
+def test_build_fail_exitcode(cli, container_runtime, ee_tag, tmp_path, data_dir):
     """Test that when a build fails, the ansible-builder exits with non-zero exit code.
 
     Example: https://github.com/ansible/ansible-builder/issues/51
     """
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'build_fail', 'execution-environment.yml')
+    bc = tmp_path
+    ee_def = data_dir / 'build_fail' / 'execution-environment.yml'
     r = cli(
         f"ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime} -v3",
         allow_error=True
@@ -18,20 +18,20 @@ def test_build_fail_exitcode(cli, container_runtime, ee_tag, tmpdir, data_dir):
     assert 'thisisnotacommand: command not found' in (r.stdout + r.stderr), (r.stdout + r.stderr)
 
 
-def test_blank_execution_environment(cli, container_runtime, ee_tag, tmpdir, data_dir):
+def test_blank_execution_environment(cli, container_runtime, ee_tag, tmp_path, data_dir):
     """Just makes sure that the buld process does not require any particular input"""
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'blank', 'execution-environment.yml')
+    bc = tmp_path
+    ee_def = data_dir / 'blank' / 'execution-environment.yml'
     cli(
         f'ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime}'
     )
     result = cli(f'{container_runtime} run --rm {ee_tag} echo "This is a simple test"')
     assert 'This is a simple test' in result.stdout, result.stdout
 
 
-def test_user_system_requirement(cli, container_runtime, ee_tag, tmpdir, data_dir):
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'subversion', 'execution-environment.yml')
+def test_user_system_requirement(cli, container_runtime, ee_tag, tmp_path, data_dir):
+    bc = tmp_path
+    ee_def = data_dir / 'subversion' / 'execution-environment.yml'
     command = f'ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime}'
     cli(command)
     result = cli(
@@ -40,9 +40,9 @@ def test_user_system_requirement(cli, container_runtime, ee_tag, tmpdir, data_di
     assert 'Subversion is a tool for version control' in result.stdout, result.stdout
 
 
-def test_collection_system_requirement(cli, container_runtime, ee_tag, tmpdir, data_dir):
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'ansible.posix.at', 'execution-environment.yml')
+def test_collection_system_requirement(cli, container_runtime, ee_tag, tmp_path, data_dir):
+    bc = tmp_path
+    ee_def = data_dir / 'ansible.posix.at' / 'execution-environment.yml'
     cli(
         f'ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime} -v3'
     )
@@ -52,9 +52,9 @@ def test_collection_system_requirement(cli, container_runtime, ee_tag, tmpdir, d
     assert 'at version' in result.stderr, result.stderr
 
 
-def test_user_python_requirement(cli, container_runtime, ee_tag, tmpdir, data_dir):
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'pip', 'execution-environment.yml')
+def test_user_python_requirement(cli, container_runtime, ee_tag, tmp_path, data_dir):
+    bc = tmp_path
+    ee_def = data_dir / 'pip' / 'execution-environment.yml'
     command = f'ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime}'
     cli(command)
     result = cli(
@@ -68,21 +68,21 @@ def test_user_python_requirement(cli, container_runtime, ee_tag, tmpdir, data_di
         assert result.rc != 0, py_library
 
 
-def test_python_git_requirement(cli, container_runtime, ee_tag, tmpdir, data_dir):
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'needs_git', 'execution-environment.yml')
+def test_python_git_requirement(cli, container_runtime, ee_tag, tmp_path, data_dir):
+    bc = tmp_path
+    ee_def = data_dir / 'needs_git' / 'execution-environment.yml'
     command = f'ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime}'
     cli(command)
     result = cli(f'{container_runtime} run --rm {ee_tag} pip3 freeze')
     assert 'flask' in result.stdout.lower(), result.stdout
 
 
-def test_prepended_steps(cli, container_runtime, ee_tag, tmpdir, data_dir):
+def test_prepended_steps(cli, container_runtime, ee_tag, tmp_path, data_dir):
     """
     Tests that prepended steps are in final stage
     """
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'prepend_steps', 'execution-environment.yml')
+    bc = tmp_path
+    ee_def = data_dir / 'prepend_steps' / 'execution-environment.yml'
     cli(
         f'ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime}'
     )
@@ -95,31 +95,31 @@ def test_prepended_steps(cli, container_runtime, ee_tag, tmpdir, data_dir):
     assert 'RUN whoami' in stages_content[-1]
 
 
-def test_build_args_basic(cli, container_runtime, ee_tag, tmpdir, data_dir):
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'build_args', 'execution-environment.yml')
+def test_build_args_basic(cli, container_runtime, ee_tag, tmp_path, data_dir):
+    bc = tmp_path
+    ee_def = data_dir / 'build_args' / 'execution-environment.yml'
     result = cli(
         f'ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime} --build-arg FOO=bar -v3'
     )
     assert 'FOO=bar' in result.stdout
 
 
-def test_build_args_from_environment(cli, container_runtime, ee_tag, tmpdir, data_dir):
+def test_build_args_from_environment(cli, container_runtime, ee_tag, tmp_path, data_dir):
     if container_runtime == 'podman':
         pytest.skip('Skipped. Podman does not support this')
 
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'build_args', 'execution-environment.yml')
+    bc = tmp_path
+    ee_def = data_dir / 'build_args' / 'execution-environment.yml'
     os.environ['FOO'] = 'secretsecret'
     result = cli(
         f'ansible-builder build -c {bc} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime} --build-arg FOO -v3'
     )
     assert 'secretsecret' in result.stdout
 
 
-def test_base_image_build_arg(cli, container_runtime, ee_tag, tmpdir, data_dir):
-    bc = str(tmpdir)
-    ee_def = os.path.join(data_dir, 'build_args', 'base-image.yml')
+def test_base_image_build_arg(cli, container_runtime, ee_tag, tmp_path, data_dir):
+    bc = tmp_path
+    ee_def = data_dir / 'build_args' / 'base-image.yml'
     os.environ['FOO'] = 'secretsecret'
 
     # Build with custom image tag, then use that as input to --build-arg EE_BASE_IMAGE
@@ -133,9 +133,9 @@ def test_base_image_build_arg(cli, container_runtime, ee_tag, tmpdir, data_dir):
 class TestPytz:
 
     @pytest.fixture(scope='class')
-    def pytz(self, cli_class, container_runtime, ee_tag_class, data_dir, tmpdir_factory):
-        bc_folder = str(tmpdir_factory.mktemp('bc'))
-        ee_def = os.path.join(data_dir, 'pytz', 'execution-environment.yml')
+    def pytz(self, cli_class, container_runtime, ee_tag_class, data_dir, tmp_path_factory):
+        bc_folder = str(tmp_path_factory.mktemp('bc'))
+        ee_def = data_dir / 'pytz' / 'execution-environment.yml'
         r = cli_class(
             f'ansible-builder build -c {bc_folder} -f {ee_def} -t {ee_tag_class} --container-runtime {container_runtime} -v 3'
         )
@@ -150,7 +150,7 @@ def test_has_pytz(self, cli, container_runtime, pytz):
 
     def test_build_layer_reuse(self, cli, container_runtime, data_dir, pytz):
         ee_tag, bc_folder = pytz
-        ee_def = os.path.join(data_dir, 'pytz', 'execution-environment.yml')
+        ee_def = data_dir / 'pytz' / 'execution-environment.yml'
         r = cli(
             f'ansible-builder build -c {bc_folder} -f {ee_def} -t {ee_tag} --container-runtime {container_runtime} -v 3'
         )

test/integration/test_introspect_cli.py

@@ -1,5 +1,4 @@
 import yaml
-import os
 
 
 def test_introspect_write(cli, data_dir):
@@ -20,37 +19,37 @@ def test_introspect_with_sanitize(cli, data_dir):
     assert '# from collection test.bindep' in r.stdout  # should have comments
 
 
-def test_introspect_write_bindep(cli, data_dir, tmpdir):
-    dest_file = os.path.join(str(tmpdir), 'req.txt')
+def test_introspect_write_bindep(cli, data_dir, tmp_path):
+    dest_file = tmp_path / 'req.txt'
     cli(f'ansible-builder introspect {data_dir} --write-bindep={dest_file}')
-    with open(dest_file, 'r') as f:
-        assert f.read() == '\n'.join([
-            'subversion [platform:rpm]  # from collection test.bindep',
-            'subversion [platform:dpkg]  # from collection test.bindep',
-            ''
-        ])
 
+    assert dest_file.read_text() == '\n'.join([
+        'subversion [platform:rpm]  # from collection test.bindep',
+        'subversion [platform:dpkg]  # from collection test.bindep',
+        ''
+    ])
 
-def test_introspect_write_python(cli, data_dir, tmpdir):
-    dest_file = os.path.join(str(tmpdir), 'req.txt')
+
+def test_introspect_write_python(cli, data_dir, tmp_path):
+    dest_file = tmp_path / 'req.txt'
     cli(f'ansible-builder introspect {data_dir} --write-pip={dest_file}')
-    with open(dest_file, 'r') as f:
-        assert f.read() == '\n'.join([
-            'pyvcloud>=14  # from collection test.metadata',
-            'pytz  # from collection test.reqfile',
-            'tacacs_plus  # from collection test.reqfile',
-            'pyvcloud>=18.0.10  # from collection test.reqfile',
-            ''
-        ])
-
-
-def test_introspect_write_python_and_sanitize(cli, data_dir, tmpdir):
-    dest_file = os.path.join(str(tmpdir), 'req.txt')
+
+    assert dest_file.read_text() == '\n'.join([
+        'pyvcloud>=14  # from collection test.metadata',
+        'pytz  # from collection test.reqfile',
+        'tacacs_plus  # from collection test.reqfile',
+        'pyvcloud>=18.0.10  # from collection test.reqfile',
+        ''
+    ])
+
+
+def test_introspect_write_python_and_sanitize(cli, data_dir, tmp_path):
+    dest_file = tmp_path / 'req.txt'
     cli(f'ansible-builder introspect {data_dir} --write-pip={dest_file} --sanitize')
-    with open(dest_file, 'r') as f:
-        assert f.read() == '\n'.join([
-            'pyvcloud>=14,>=18.0.10  # from collection test.metadata,test.reqfile',
-            'pytz  # from collection test.reqfile',
-            'tacacs_plus  # from collection test.reqfile',
-            ''
-        ])
+
+    assert dest_file.read_text() == '\n'.join([
+        'pyvcloud>=14,>=18.0.10  # from collection test.metadata,test.reqfile',
+        'pytz  # from collection test.reqfile',
+        'tacacs_plus  # from collection test.reqfile',
+        ''
+    ])