From 1cfb0609a74281afdd5ae36a1b4140979b4857b9 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Thu, 14 Oct 2021 18:06:25 -0400 Subject: [PATCH] Fix unit tests to run consistently in CI and locally (#815) Use pytest-mock and tmp_path fixtures Mock test environment variables to prevent inconsistent test results Reformat test fixtures so they are easier to read --- test/integration/conftest.py | 6 +- test/integration/test_display_callback.py | 13 +-- test/integration/test_main.py | 23 +++- .../test_transmit_worker_process.py | 39 ++++--- test/unit/config/test__base.py | 21 ++-- test/unit/config/test_ansible_cfg.py | 56 ++++++---- test/unit/config/test_command.py | 58 ++++++---- test/unit/config/test_doc.py | 101 ++++++++++++------ test/unit/config/test_inventory.py | 62 +++++++---- test/unit/config/test_runner.py | 17 ++- test/unit/test_runner.py | 6 +- test/unit/test_utils.py | 62 ++++++----- 12 files changed, 293 insertions(+), 171 deletions(-) diff --git a/test/integration/conftest.py b/test/integration/conftest.py index fe8a2ecd5..ed78f9ddb 100644 --- a/test/integration/conftest.py +++ b/test/integration/conftest.py @@ -7,14 +7,14 @@ @pytest.fixture(scope='function') -def rc(tmpdir): - rc = RunnerConfig(str(tmpdir)) +def rc(tmp_path): + rc = RunnerConfig(str(tmp_path)) rc.suppress_ansible_output = True rc.expect_passwords = { pexpect.TIMEOUT: None, pexpect.EOF: None } - rc.cwd = str(tmpdir) + rc.cwd = str(tmp_path) rc.env = {} rc.job_timeout = 10 rc.idle_timeout = 0 diff --git a/test/integration/test_display_callback.py b/test/integration/test_display_callback.py index 58cd01e4f..255bcb95c 100644 --- a/test/integration/test_display_callback.py +++ b/test/integration/test_display_callback.py @@ -13,8 +13,9 @@ @pytest.fixture() -def executor(tmpdir, request, is_pre_ansible28): - private_data_dir = six.text_type(tmpdir.mkdir('foo')) +def executor(tmp_path, request, is_pre_ansible28): + private_data_dir = tmp_path / 'foo' + private_data_dir.mkdir() playbooks = request.node.callspec.params.get('playbook') playbook = list(playbooks.values())[0] @@ -317,7 +318,7 @@ def test_module_level_no_log(executor, playbook, skipif_pre_ansible28): assert 'SENSITIVE' not in json.dumps(list(executor.events)) -def test_output_when_given_invalid_playbook(tmpdir): +def test_output_when_given_invalid_playbook(tmp_path): # As shown in the following issue: # # https://github.com/ansible/ansible-runner/issues/29 @@ -328,7 +329,7 @@ def test_output_when_given_invalid_playbook(tmpdir): # https://github.com/ansible/ansible-runner/pull/34 # # But no test validated it. This does that. - private_data_dir = str(tmpdir) + private_data_dir = str(tmp_path) executor = init_runner( private_data_dir=private_data_dir, inventory="localhost ansible_connection=local", @@ -342,7 +343,7 @@ def test_output_when_given_invalid_playbook(tmpdir): assert "could not be found" in stdout -def test_output_when_given_non_playbook_script(tmpdir): +def test_output_when_given_non_playbook_script(tmp_path): # As shown in the following pull request: # # https://github.com/ansible/ansible-runner/pull/256 @@ -354,7 +355,7 @@ def test_output_when_given_non_playbook_script(tmpdir): # this is a retro-active test based on the sample repo provided in the PR: # # https://github.com/AlanCoding/ansible-runner-examples/tree/master/non_playbook/sleep_with_writes - private_data_dir = str(tmpdir) + private_data_dir = str(tmp_path) with open(os.path.join(private_data_dir, "args"), 'w') as args_file: args_file.write("bash sleep_and_write.sh\n") with open(os.path.join(private_data_dir, "sleep_and_write.sh"), 'w') as script_file: diff --git a/test/integration/test_main.py b/test/integration/test_main.py index 5c073f276..ce09657fc 100644 --- a/test/integration/test_main.py +++ b/test/integration/test_main.py @@ -124,11 +124,10 @@ def test_module_run_debug(tmp_path): assert rc == 0 -def test_module_run_clean(): - with temp_directory() as temp_dir: - rc = main(['run', '-m', 'ping', - '--hosts', 'localhost', - temp_dir]) +def test_module_run_clean(tmp_path): + rc = main(['run', '-m', 'ping', + '--hosts', 'localhost', + str(tmp_path)]) assert rc == 0 @@ -155,6 +154,9 @@ def test_role_run_abs(tmp_path): assert rc == 0 +# FIXME: This test interferes with test_role_logfile_abs. Marking it as serial so it is executed +# in a separate test run. +@pytest.mark.serial def test_role_logfile(skipif_pre_ansible28, tmp_path): logfile = tmp_path.joinpath('test_role_logfile') rc = main(['run', '-r', 'benthomasson.hello_role', @@ -167,6 +169,17 @@ def test_role_logfile(skipif_pre_ansible28, tmp_path): assert rc == 0 +def test_role_logfile_abs(tmp_path): + rc = main(['run', '-r', 'benthomasson.hello_role', + '--hosts', 'localhost', + '--roles-path', os.path.join(HERE, 'project/roles'), + '--logfile', tmp_path.joinpath('new_logfile').as_posix(), + str(tmp_path)]) + + assert tmp_path.joinpath('new_logfile').exists() + assert rc == 0 + + def test_role_bad_project_dir(): with open("bad_project_dir", 'w') as f: diff --git a/test/integration/test_transmit_worker_process.py b/test/integration/test_transmit_worker_process.py index 977a8d67b..78e5f5faa 100644 --- a/test/integration/test_transmit_worker_process.py +++ b/test/integration/test_transmit_worker_process.py @@ -53,10 +53,14 @@ def check_artifacts(self, process_dir, job_type): assert '"ansible_facts"' in stdout @pytest.mark.parametrize("job_type", ['run', 'adhoc']) - def test_remote_job_interface(self, tmpdir, test_data_dir, job_type): + def test_remote_job_interface(self, tmp_path, test_data_dir, job_type): transmit_dir = os.path.join(test_data_dir, 'debug') - worker_dir = str(tmpdir.mkdir('for_worker')) - process_dir = str(tmpdir.mkdir('for_process')) + worker_dir = tmp_path / 'for_worker' + worker_dir.mkdir() + + process_dir = tmp_path / 'for_process' + process_dir.mkdir() + job_kwargs = self.get_job_kwargs(job_type) outgoing_buffer = tempfile.NamedTemporaryFile() @@ -90,17 +94,21 @@ def test_remote_job_interface(self, tmpdir, test_data_dir, job_type): processor = Processor(_input=incoming_buffer, private_data_dir=process_dir) processor.run() - self.check_artifacts(process_dir, job_type) + self.check_artifacts(str(process_dir), job_type) @pytest.mark.parametrize("job_type", ['run', 'adhoc']) - def test_remote_job_by_sockets(self, tmpdir, test_data_dir, container_runtime_installed, job_type): + def test_remote_job_by_sockets(self, tmp_path, test_data_dir, container_runtime_installed, job_type): """This test case is intended to be close to how the AWX use case works the process interacts with receptorctl with sockets sockets are used here, but worker is manually called instead of invoked by receptor """ transmit_dir = os.path.join(test_data_dir, 'debug') - worker_dir = str(tmpdir.mkdir('for_worker')) - process_dir = str(tmpdir.mkdir('for_process')) + worker_dir = tmp_path / 'for_worker' + worker_dir.mkdir() + + process_dir = tmp_path / 'for_process' + process_dir.mkdir() + job_kwargs = self.get_job_kwargs(job_type) def transmit_method(transmit_sockfile_write): @@ -155,7 +163,7 @@ def process_method(results_sockfile_read): assert set(os.listdir(worker_dir)) == {'artifacts', 'inventory', 'project', 'env'} - self.check_artifacts(process_dir, job_type) + self.check_artifacts(str(process_dir), job_type) def test_missing_private_dir_transmit(tmpdir): @@ -173,8 +181,9 @@ def test_missing_private_dir_transmit(tmpdir): assert "private_data_dir path is either invalid or does not exist" in str(excinfo.value) -def test_garbage_private_dir_worker(tmpdir): - worker_dir = str(tmpdir.mkdir('for_worker')) +def test_garbage_private_dir_worker(tmp_path): + worker_dir = tmp_path / 'for_worker' + worker_dir.mkdir() incoming_buffer = io.BytesIO( b'{"kwargs": {"playbook": "debug.yml"}}\n{"zipfile": 5}\n\x01\x02\x03\x04\x05{"eof": true}\n') outgoing_buffer = io.BytesIO() @@ -190,8 +199,9 @@ def test_garbage_private_dir_worker(tmpdir): assert b'"status": "error"' in sent -def test_unparsable_private_dir_worker(tmpdir): - worker_dir = str(tmpdir.mkdir('for_worker')) +def test_unparsable_private_dir_worker(tmp_path): + worker_dir = tmp_path / 'for_worker' + worker_dir.mkdir() incoming_buffer = io.BytesIO(b'') outgoing_buffer = io.BytesIO() @@ -206,8 +216,9 @@ def test_unparsable_private_dir_worker(tmpdir): assert b'"status": "error"' in sent -def test_unparsable_private_dir_processor(tmpdir): - process_dir = str(tmpdir.mkdir('for_process')) +def test_unparsable_private_dir_processor(tmp_path): + process_dir = tmp_path / 'for_process' + process_dir.mkdir() incoming_buffer = io.BytesIO(b'') processor = run( diff --git a/test/unit/config/test__base.py b/test/unit/config/test__base.py index 449f76f93..9133a62a9 100644 --- a/test/unit/config/test__base.py +++ b/test/unit/config/test__base.py @@ -280,7 +280,7 @@ def test_container_volume_mounting_with_Z(tmp_path, mocker): @pytest.mark.parametrize('container_runtime', ['docker', 'podman']) -def test_containerization_settings(tmpdir, container_runtime): +def test_containerization_settings(tmp_path, container_runtime, mocker): mocker.patch.dict('os.environ', {'HOME': str(tmp_path)}, clear=True) tmp_path.joinpath('.ssh').mkdir() @@ -317,10 +317,8 @@ def test_containerization_settings(tmpdir, container_runtime): '-v', '{}/.ssh/:/home/runner/.ssh/'.format(str(tmp_path)), ] - expected_command_start = [container_runtime, 'run', '--rm', '--tty', '--interactive', '--workdir', '/runner/project'] + \ - ['-v', '{}/.ssh/:/home/runner/.ssh/'.format(os.environ['HOME'])] if container_runtime == 'podman': - expected_command_start += ['--group-add=root', '--ipc=host'] + expected_command_start.extend(['--group-add=root', '--ipc=host']) expected_command_start.extend([ '-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir), @@ -330,19 +328,12 @@ def test_containerization_settings(tmpdir, container_runtime): expected_command_start.extend(extra_container_args) - expected_command_start += ['-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir)] + \ - ['-v', '{}/:/runner/:Z'.format(rc.private_data_dir)] + \ - ['--env-file', '{}/env.list'.format(rc.artifact_dir)] + \ - extra_container_args + \ - ['--name', 'ansible_runner_foo'] + \ - ['my_container', 'ansible-playbook', 'main.yaml', '-i', '/tmp/inventory'] + expected_command_start.extend([ + '--name', 'ansible_runner_foo', + 'my_container', 'ansible-playbook', 'main.yaml', '-i', '/tmp/inventory', ]) - for index, element in enumerate(expected_command_start): - if '--user=' in element: - assert '--user=' in rc.command[index] - else: - assert rc.command[index] == element + assert expected_command_start == rc.command @pytest.mark.parametrize( diff --git a/test/unit/config/test_ansible_cfg.py b/test/unit/config/test_ansible_cfg.py index f79dfc185..0048d9d2a 100644 --- a/test/unit/config/test_ansible_cfg.py +++ b/test/unit/config/test_ansible_cfg.py @@ -55,9 +55,12 @@ def test_prepare_config_invalid_action(): @pytest.mark.parametrize('container_runtime', ['docker', 'podman']) -def test_prepare_config_command_with_containerization(tmpdir, container_runtime): +def test_prepare_config_command_with_containerization(tmp_path, container_runtime, mocker): + mocker.patch.dict('os.environ', {'HOME': str(tmp_path)}, clear=True) + tmp_path.joinpath('.ssh').mkdir() + kwargs = { - 'private_data_dir': tmpdir, + 'private_data_dir': tmp_path, 'process_isolation': True, 'container_image': 'my_container', 'process_isolation_executable': container_runtime @@ -71,22 +74,37 @@ def test_prepare_config_command_with_containerization(tmpdir, container_runtime) if container_runtime == 'podman': extra_container_args = ['--quiet'] else: - extra_container_args = ['--user={os.getuid()}'] + extra_container_args = [f'--user={os.getuid()}'] + + expected_command_start = [ + container_runtime, + 'run', + '--rm', + '--interactive', + '--workdir', + '/runner/project', + '-v', '{}/.ssh/:/home/runner/.ssh/'.format(rc.private_data_dir), + ] - expected_command_start = [container_runtime, 'run', '--rm', '--interactive', '--workdir', '/runner/project'] + \ - ['-v', '{}/.ssh/:/home/runner/.ssh/'.format(os.environ['HOME'])] if container_runtime == 'podman': - expected_command_start += ['--group-add=root', '--ipc=host'] - - expected_command_start += ['-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir)] + \ - ['-v', '{}/:/runner/:Z'.format(rc.private_data_dir)] + \ - ['--env-file', '{}/env.list'.format(rc.artifact_dir)] + \ - extra_container_args + \ - ['--name', 'ansible_runner_foo'] + \ - ['my_container', 'ansible-config', 'list', '-c', '/tmp/ansible.cfg'] - - for index, element in enumerate(expected_command_start): - if '--user=' in element: - assert '--user=' in rc.command[index] - else: - assert rc.command[index] == element + expected_command_start.extend(['--group-add=root', '--ipc=host']) + + expected_command_start.extend([ + '-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir), + '-v', '{}/:/runner/:Z'.format(rc.private_data_dir), + '--env-file', '{}/env.list'.format(rc.artifact_dir), + ]) + + expected_command_start.extend(extra_container_args) + + expected_command_start.extend([ + '--name', + 'ansible_runner_foo', + 'my_container', + 'ansible-config', + 'list', + '-c', + '/tmp/ansible.cfg', + ]) + + assert expected_command_start == rc.command diff --git a/test/unit/config/test_command.py b/test/unit/config/test_command.py index 5a7996a16..70e948d5c 100644 --- a/test/unit/config/test_command.py +++ b/test/unit/config/test_command.py @@ -64,9 +64,12 @@ def test_prepare_run_command_generic(): @pytest.mark.parametrize('container_runtime', ['docker', 'podman']) -def test_prepare_run_command_with_containerization(tmpdir, container_runtime): +def test_prepare_run_command_with_containerization(tmp_path, container_runtime, mocker): + mocker.patch.dict('os.environ', {'HOME': str(tmp_path)}, clear=True) + tmp_path.joinpath('.ssh').mkdir() + kwargs = { - 'private_data_dir': tmpdir, + 'private_data_dir': tmp_path, 'process_isolation': True, 'container_image': 'my_container', 'process_isolation_executable': container_runtime @@ -83,23 +86,38 @@ def test_prepare_run_command_with_containerization(tmpdir, container_runtime): if container_runtime == 'podman': extra_container_args = ['--quiet'] else: - extra_container_args = ['--user={os.getuid()}'] - - expected_command_start = [container_runtime, 'run', '--rm', '--tty', '--interactive', '--workdir', '/runner/project'] + \ - ['-v', '{}/:{}/'.format(cwd, cwd), '-v', '{}/.ssh/:/home/runner/.ssh/'.format(os.environ['HOME'])] + extra_container_args = [f'--user={os.getuid()}'] + + expected_command_start = [ + container_runtime, + 'run', + '--rm', + '--tty', + '--interactive', + '--workdir', + '/runner/project', + '-v', '{}/:{}/'.format(cwd, cwd), + '-v', '{}/.ssh/:/home/runner/.ssh/'.format(rc.private_data_dir), + ] if container_runtime == 'podman': - expected_command_start += ['--group-add=root', '--ipc=host'] - - expected_command_start += ['-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir)] + \ - ['-v', '{}/:/runner/:Z'.format(rc.private_data_dir)] + \ - ['--env-file', '{}/env.list'.format(rc.artifact_dir)] + \ - extra_container_args + \ - ['--name', 'ansible_runner_foo'] + \ - ['my_container'] + [executable_cmd] + cmdline_args - - for index, element in enumerate(expected_command_start): - if '--user=' in element: - assert '--user=' in rc.command[index] - else: - assert rc.command[index] == element + expected_command_start.extend(['--group-add=root', '--ipc=host']) + + expected_command_start.extend([ + '-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir), + '-v', '{}/:/runner/:Z'.format(rc.private_data_dir), + '--env-file', '{}/env.list'.format(rc.artifact_dir), + ]) + + expected_command_start.extend(extra_container_args) + + expected_command_start.extend([ + '--name', + 'ansible_runner_foo', + 'my_container', + executable_cmd, + ]) + + expected_command_start.extend(cmdline_args) + + assert expected_command_start == rc.command diff --git a/test/unit/config/test_doc.py b/test/unit/config/test_doc.py index 1efc6d143..5b3b06afe 100755 --- a/test/unit/config/test_doc.py +++ b/test/unit/config/test_doc.py @@ -61,9 +61,12 @@ def test_prepare_plugin_docs_command(): @pytest.mark.parametrize('container_runtime', ['docker', 'podman']) -def test_prepare_plugin_docs_command_with_containerization(tmpdir, container_runtime): +def test_prepare_plugin_docs_command_with_containerization(tmp_path, container_runtime, mocker): + mocker.patch.dict('os.environ', {'HOME': str(tmp_path)}, clear=True) + tmp_path.joinpath('.ssh').mkdir() + kwargs = { - 'private_data_dir': tmpdir, + 'private_data_dir': tmp_path, 'process_isolation': True, 'container_image': 'my_container', 'process_isolation_executable': container_runtime @@ -81,25 +84,41 @@ def test_prepare_plugin_docs_command_with_containerization(tmpdir, container_run if container_runtime == 'podman': extra_container_args = ['--quiet'] else: - extra_container_args = ['--user={os.getuid()}'] + extra_container_args = [f'--user={os.getuid()}'] + + expected_command_start = [ + container_runtime, + 'run', + '--rm', + '--interactive', + '--workdir', + '/runner/project', + '-v', '{}/.ssh/:/home/runner/.ssh/'.format(rc.private_data_dir), + ] - expected_command_start = [container_runtime, 'run', '--rm', '--interactive', '--workdir', '/runner/project'] + \ - ['-v', '{}/.ssh/:/home/runner/.ssh/'.format(os.environ['HOME'])] if container_runtime == 'podman': - expected_command_start += ['--group-add=root', '--ipc=host'] + expected_command_start.extend(['--group-add=root', '--ipc=host']) - expected_command_start += ['-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir)] + \ - ['-v', '{}/:/runner/:Z'.format(rc.private_data_dir)] + \ - ['--env-file', '{}/env.list'.format(rc.artifact_dir)] + \ - extra_container_args + \ - ['--name', 'ansible_runner_foo'] + \ - ['my_container'] + ['ansible-doc', '-s', '-t', 'module', '--playbook-dir', '/tmp/test', 'copy file'] + expected_command_start.extend([ + '-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir), + '-v', '{}/:/runner/:Z'.format(rc.private_data_dir), + '--env-file', '{}/env.list'.format(rc.artifact_dir), + ]) - for index, element in enumerate(expected_command_start): - if '--user=' in element: - assert '--user=' in rc.command[index] - else: - assert rc.command[index] == element + expected_command_start.extend(extra_container_args) + + expected_command_start.extend([ + '--name', 'ansible_runner_foo', + 'my_container', + 'ansible-doc', + '-s', + '-t', 'module', + '--playbook-dir', '/tmp/test', + 'copy ' + 'file', + ]) + + assert expected_command_start == rc.command def test_prepare_plugin_list_command(): @@ -112,9 +131,12 @@ def test_prepare_plugin_list_command(): @pytest.mark.parametrize('container_runtime', ['docker', 'podman']) -def test_prepare_plugin_list_command_with_containerization(tmpdir, container_runtime): +def test_prepare_plugin_list_command_with_containerization(tmp_path, container_runtime, mocker): + mocker.patch.dict('os.environ', {'HOME': str(tmp_path)}, clear=True) + tmp_path.joinpath('.ssh').mkdir() + kwargs = { - 'private_data_dir': tmpdir, + 'private_data_dir': tmp_path, 'process_isolation': True, 'container_image': 'my_container', 'process_isolation_executable': container_runtime @@ -129,19 +151,38 @@ def test_prepare_plugin_list_command_with_containerization(tmpdir, container_run if container_runtime == 'podman': extra_container_args = ['--quiet'] else: - extra_container_args = ['--user={os.getuid()}'] + extra_container_args = [f'--user={os.getuid()}'] + + expected_command_start = [ + container_runtime, + 'run', + '--rm', + '--interactive', + '--workdir', + '/runner/project', + '-v', '{}/.ssh/:/home/runner/.ssh/'.format(rc.private_data_dir), + ] - expected_command_start = [container_runtime, 'run', '--rm', '--interactive', '--workdir', '/runner/project'] + \ - ['-v', '{}/.ssh/:/home/runner/.ssh/'.format(os.environ['HOME'])] if container_runtime == 'podman': - expected_command_start += ['--group-add=root', '--ipc=host'] - - expected_command_start += ['-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir)] + \ - ['-v', '{}/:/runner/:Z'.format(rc.private_data_dir)] + \ - ['--env-file', '{}/env.list'.format(rc.artifact_dir)] + \ - extra_container_args + \ - ['--name', 'ansible_runner_foo'] + \ - ['my_container'] + ['ansible-doc', '-F', '-t', 'module', '--playbook-dir', '/tmp/test', '-M', '/test/module'] + expected_command_start.extend(['--group-add=root', '--ipc=host']) + + expected_command_start.extend([ + '-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir), + '-v', '{}/:/runner/:Z'.format(rc.private_data_dir), + '--env-file', '{}/env.list'.format(rc.artifact_dir), + ]) + + expected_command_start.extend(extra_container_args) + + expected_command_start.extend([ + '--name', 'ansible_runner_foo', + 'my_container', + 'ansible-doc', + '-F', + '-t', 'module', + '--playbook-dir', '/tmp/test', + '-M', '/test/module' + ]) for index, element in enumerate(expected_command_start): if '--user=' in element: diff --git a/test/unit/config/test_inventory.py b/test/unit/config/test_inventory.py index 624425fec..8af2d51d9 100644 --- a/test/unit/config/test_inventory.py +++ b/test/unit/config/test_inventory.py @@ -87,9 +87,12 @@ def test_prepare_inventory_invalid_graph_response_format(): @pytest.mark.parametrize('container_runtime', ['docker', 'podman']) -def test_prepare_inventory_command_with_containerization(tmpdir, container_runtime): +def test_prepare_inventory_command_with_containerization(tmp_path, container_runtime, mocker): + mocker.patch.dict('os.environ', {'HOME': str(tmp_path)}, clear=True) + tmp_path.joinpath('.ssh').mkdir() + kwargs = { - 'private_data_dir': tmpdir, + 'private_data_dir': tmp_path, 'process_isolation': True, 'container_image': 'my_container', 'process_isolation_executable': container_runtime @@ -106,26 +109,45 @@ def test_prepare_inventory_command_with_containerization(tmpdir, container_runti if container_runtime == 'podman': extra_container_args = ['--quiet'] else: - extra_container_args = ['--user={os.getuid()}'] + extra_container_args = [f'--user={os.getuid()}'] + + expected_command_start = [ + container_runtime, + 'run', + '--rm', + '--interactive', + '--workdir', + '/runner/project', + '-v', '{}/.ssh/:/home/runner/.ssh/'.format(rc.private_data_dir), + ] - expected_command_start = [container_runtime, 'run', '--rm', '--interactive', '--workdir', '/runner/project'] + \ - ['-v', '{}/.ssh/:/home/runner/.ssh/'.format(os.environ['HOME'])] if container_runtime == 'podman': - expected_command_start += ['--group-add=root', '--ipc=host'] - - expected_command_start += ['-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir)] + \ - ['-v', '{}/:/runner/:Z'.format(rc.private_data_dir)] + \ - ['--env-file', '{}/env.list'.format(rc.artifact_dir)] + \ - extra_container_args + \ - ['--name', 'ansible_runner_foo', 'my_container'] + \ - ['ansible-inventory', '--list', '-i', '/tmp/inventory1', '-i', '/tmp/inventory2', '--yaml', '--playbook-dir'] + \ - ['/tmp', '--vault-id', '1234', '--vault-password-file', '/tmp/password', '--output', '/tmp/inv_out.txt'] + \ - ['--export'] + expected_command_start.extend(['--group-add=root', '--ipc=host']) + + expected_command_start.extend([ + '-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir), + '-v', '{}/:/runner/:Z'.format(rc.private_data_dir), + '--env-file', '{}/env.list'.format(rc.artifact_dir), + ]) + + expected_command_start.extend(extra_container_args) + + expected_command_start.extend([ + '--name', + 'ansible_runner_foo', + 'my_container', + 'ansible-inventory', + '--list', + '-i', '/tmp/inventory1', + '-i', '/tmp/inventory2', + '--yaml', + '--playbook-dir', '/tmp', + '--vault-id', '1234', + '--vault-password-file', '/tmp/password', + '--output', '/tmp/inv_out.txt', + '--export', + ]) assert len(expected_command_start) == len(rc.command) - for index, element in enumerate(expected_command_start): - if '--user=' in element: - assert '--user=' in rc.command[index] - else: - assert rc.command[index] == element + assert expected_command_start == rc.command diff --git a/test/unit/config/test_runner.py b/test/unit/config/test_runner.py index e042ab0a7..6c03da5c4 100644 --- a/test/unit/config/test_runner.py +++ b/test/unit/config/test_runner.py @@ -649,10 +649,11 @@ def test_profiling_plugin_settings(mocker): '--sticky', '-g', 'cpuacct,memory,pids:ansible-runner/{}'.format(rc.ident), - 'ansible-playbook' + 'ansible-playbook', + 'main.yaml', ] - for index, element in enumerate(expected_command_start): - assert rc.command[index] == element + + assert expected_command_start == rc.command assert 'main.yaml' in rc.command assert rc.env['ANSIBLE_CALLBACK_WHITELIST'] == 'cgroup_perf_recap' assert rc.env['CGROUP_CONTROL_GROUP'] == 'ansible-runner/{}'.format(rc.ident) @@ -687,7 +688,7 @@ def test_container_volume_mounting_with_Z(mocker, tmp_path): mocker.patch('os.path.isdir', return_value=True) mocker.patch('os.path.exists', return_value=True) - rc = RunnerConfig(str(tmpdir)) + rc = RunnerConfig(str(tmp_path)) rc.container_volume_mounts = ['/tmp/project_path:/tmp/project_path:Z'] rc.container_name = 'foo' rc.env = {} @@ -723,7 +724,7 @@ def test_containerization_settings(tmp_path, container_runtime, mocker): if container_runtime == 'podman': extra_container_args = ['--quiet'] else: - extra_container_args = ['--user={os.getuid()}'] + extra_container_args = [f'--user={os.getuid()}'] expected_command_start = [container_runtime, 'run', '--rm', '--tty', '--interactive', '--workdir', '/runner/project'] + \ ['-v', '{}/:/runner/:Z'.format(rc.private_data_dir)] + \ @@ -733,8 +734,4 @@ def test_containerization_settings(tmp_path, container_runtime, mocker): ['--name', 'ansible_runner_foo'] + \ ['my_container', 'ansible-playbook', '-i', '/runner/inventory/hosts', 'main.yaml'] - for index, element in enumerate(expected_command_start): - if '--user' in element: - assert '--user=' in rc.command[index] - else: - assert rc.command[index] == element + assert expected_command_start == rc.command diff --git a/test/unit/test_runner.py b/test/unit/test_runner.py index 70bbcc67d..8a66d982e 100644 --- a/test/unit/test_runner.py +++ b/test/unit/test_runner.py @@ -17,14 +17,14 @@ @pytest.fixture(scope='function') -def rc(request, tmpdir): - rc = RunnerConfig(str(tmpdir)) +def rc(request, tmp_path): + rc = RunnerConfig(str(tmp_path)) rc.suppress_ansible_output = True rc.expect_passwords = { pexpect.TIMEOUT: None, pexpect.EOF: None } - rc.cwd = str(tmpdir) + rc.cwd = str(tmp_path) rc.env = {} rc.job_timeout = .5 rc.idle_timeout = 0 diff --git a/test/unit/test_utils.py b/test/unit/test_utils.py index f8124f0e4..b18e573b9 100644 --- a/test/unit/test_utils.py +++ b/test/unit/test_utils.py @@ -1,9 +1,11 @@ import json +import os import shutil import tempfile -import os import io +from pathlib import Path + import pytest from ansible_runner.utils import ( @@ -235,29 +237,34 @@ def test_sanitize_container_name(container_name, expected_name): @pytest.mark.parametrize('symlink_dest,check_content', [ - ('/proc/cpuinfo', []), + ('/bin', []), ('ordinary_file.txt', ['my_link']), ('ordinary_directory', ['my_link/dir_file.txt']), ('.', ['my_link/ordinary_directory/dir_file.txt', 'my_link/my_link/ordinary_file.txt']), ('filedoesnotexist.txt', []) ], ids=['global', 'local', 'directory', 'recursive', 'bad']) -def test_transmit_symlink(tmpdir, symlink_dest, check_content): +def test_transmit_symlink(tmp_path, symlink_dest, check_content): + symlink_dest = Path(symlink_dest) + # prepare the input private_data_dir directory to zip - pdd = tmpdir.mkdir('symlink_zip_test') + pdd = tmp_path / 'symlink_zip_test' + pdd.mkdir() # Create some basic shared demo content - with open(os.path.join(pdd, 'ordinary_file.txt'), 'w') as f: + with open(pdd / 'ordinary_file.txt', 'w') as f: f.write('hello world') - os.mkdir(os.path.join(pdd, 'ordinary_directory')) - with open(os.path.join(pdd, 'ordinary_directory', 'dir_file.txt'), 'w') as f: + + ord_dir = pdd / 'ordinary_directory' + ord_dir.mkdir() + with open(ord_dir / 'dir_file.txt', 'w') as f: f.write('hello world') - old_symlink_path = os.path.join(pdd, 'my_link') - os.symlink(symlink_dest, old_symlink_path) + old_symlink_path = pdd / 'my_link' + old_symlink_path.symlink_to(symlink_dest) # SANITY - set expectations for the symlink - assert os.path.islink(old_symlink_path) - os.readlink(old_symlink_path) == symlink_dest + assert old_symlink_path.is_symlink() + assert os.readlink(old_symlink_path) == str(symlink_dest) # zip and stream the data into the in-memory buffer outgoing_buffer outgoing_buffer = io.BytesIO() @@ -265,7 +272,8 @@ def test_transmit_symlink(tmpdir, symlink_dest, check_content): stream_dir(pdd, outgoing_buffer) # prepare the destination private_data_dir to transmit to - dest_dir = tmpdir.mkdir('symlink_zip_dest') + dest_dir = tmp_path / 'symlink_zip_dest' + dest_dir.mkdir() # Extract twice so we assure that existing data does not break things for i in range(2): @@ -277,13 +285,13 @@ def test_transmit_symlink(tmpdir, symlink_dest, check_content): unstream_dir(outgoing_buffer, size_data['zipfile'], dest_dir) # Assure the new symlink is still the same type of symlink - new_symlink_path = os.path.join(dest_dir, 'my_link') - assert os.path.islink(new_symlink_path) - os.readlink(new_symlink_path) == symlink_dest + new_symlink_path = dest_dir / 'my_link' + assert new_symlink_path.is_symlink() + assert os.readlink(new_symlink_path) == str(symlink_dest) for fname in check_content: - abs_path = os.path.join(dest_dir, fname) - assert os.path.exists(abs_path), f'Expected "{fname}" in target dir to be a file with content.' + abs_path = dest_dir / fname + assert abs_path.exists(), f'Expected "{fname}" in target dir to be a file with content.' with open(abs_path, 'r') as f: assert f.read() == 'hello world' @@ -294,23 +302,25 @@ def test_transmit_symlink(tmpdir, symlink_dest, check_content): 0o555, 0o700, ]) -def test_transmit_permissions(tmpdir, fperm): - - pdd = tmpdir.mkdir('transmit_permission_test') +def test_transmit_permissions(tmp_path, fperm): + # breakpoint() + pdd = tmp_path / 'transmit_permission_test' + pdd.mkdir() - old_file_path = os.path.join(pdd, 'ordinary_file.txt') + old_file_path = pdd / 'ordinary_file.txt' with open(old_file_path, 'w') as f: f.write('hello world') - os.chmod(old_file_path, fperm) + old_file_path.chmod(fperm) # SANITY - set expectations for the file - assert oct(os.stat(old_file_path).st_mode & 0o777) == oct(fperm) + # assert oct(os.stat(old_file_path).st_mode & 0o777) == oct(fperm) + assert oct(old_file_path.stat().st_mode & 0o777) == oct(fperm) outgoing_buffer = io.BytesIO() outgoing_buffer.name = 'not_stdout' stream_dir(pdd, outgoing_buffer) - dest_dir = tmpdir.mkdir('transmit_permission_dest') + dest_dir = tmp_path / 'transmit_permission_dest' outgoing_buffer.seek(0) first_line = outgoing_buffer.readline() @@ -318,5 +328,5 @@ def test_transmit_permissions(tmpdir, fperm): unstream_dir(outgoing_buffer, size_data['zipfile'], dest_dir) # Assure the new file is the same permissions - new_file_path = os.path.join(dest_dir, 'ordinary_file.txt') - assert oct(os.stat(new_file_path).st_mode) == oct(os.stat(old_file_path).st_mode) + new_file_path = dest_dir / 'ordinary_file.txt' + assert oct(new_file_path.stat().st_mode) == oct(old_file_path.stat().st_mode)