From 44ac34fa93185334603c79564876a547de723a5b Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 15:37:52 -0400 Subject: [PATCH 01/25] tests: Add config tests subclasses --- tests/test_config.py | 87 ++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 48 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 65aa924..3f1eca8 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -57,10 +57,13 @@ def test_script_key_mapping_invalid(self) -> None: @pytest.mark.usefixtures("in_tmp_path") -class TestConfig: - ###################################################################### - # Find config +class ConfigTest: + def _invalid_config(self, match=None): + with pytest.raises(scuba.config.ConfigError, match=match) as e: + load_config() + +class TestFindConfig(ConfigTest): def test_find_config_cur_dir(self, in_tmp_path) -> None: """find_config can find the config in the current directory""" with open(".scuba.yml", "w") as f: @@ -108,13 +111,8 @@ def test_find_config_nonexist(self) -> None: with pytest.raises(scuba.config.ConfigError): scuba.config.find_config() - ###################################################################### - # Load config - - def _invalid_config(self, match=None): - with pytest.raises(scuba.config.ConfigError, match=match) as e: - load_config() +class TestLoadConfig(ConfigTest): def test_load_config_no_image(self) -> None: """load_config raises ConfigError if the config is empty and image is referenced""" with open(".scuba.yml", "w") as f: @@ -288,9 +286,8 @@ def test_load_config_safe_external(self) -> None: self.__test_load_config_safe(".external.yml") - ############################################################################ - # Hooks +class TestConfigHooks(ConfigTest): def test_hooks_mixed(self) -> None: """hooks of mixed forms are valid""" with open(".scuba.yml", "w") as f: @@ -343,9 +340,8 @@ def test_hooks_missing_script(self) -> None: self._invalid_config() - ############################################################################ - # Env +class TestConfigEnv(ConfigTest): def test_env_invalid(self) -> None: """Environment must be dict or list of strings""" with open(".scuba.yml", "w") as f: @@ -451,9 +447,8 @@ def test_env_alias(self) -> None: MORE="Hello world", ) - ############################################################################ - # Entrypoint +class TestConfigEntrypoint(ConfigTest): def test_entrypoint_not_set(self) -> None: """Entrypoint can be missing""" with open(".scuba.yml", "w") as f: @@ -571,9 +566,8 @@ def test_alias_entrypoint(self) -> None: config = load_config() assert config.aliases["testalias"].entrypoint == "use_this_ep" - ############################################################################ - # docker_args +class TestConfigDockerArgs(ConfigTest): def test_docker_args_not_set(self) -> None: """docker_args can be missing""" with open(".scuba.yml", "w") as f: @@ -794,10 +788,9 @@ def test_alias_docker_args_from_yaml_override(self) -> None: config.aliases["testalias"].docker_args, scuba.config.OverrideMixin ) - ############################################################################ - # volumes - def test_volumes_not_set(self) -> None: +class TestConfigVolumes(ConfigTest): + def test_not_set(self) -> None: """volumes can be missing""" with open(".scuba.yml", "w") as f: f.write( @@ -809,7 +802,7 @@ def test_volumes_not_set(self) -> None: config = load_config() assert config.volumes is None - def test_volumes_null(self) -> None: + def test_null(self) -> None: """volumes can be set to null""" with open(".scuba.yml", "w") as f: f.write( @@ -822,8 +815,8 @@ def test_volumes_null(self) -> None: config = load_config() assert config.volumes == None - def test_volumes_invalid(self) -> None: - """volumes of incorrect type raises ConfigError""" + def test_invalid_int(self) -> None: + """volumes of incorrect type (int) raises ConfigError""" with open(".scuba.yml", "w") as f: f.write( r""" @@ -834,7 +827,7 @@ def test_volumes_invalid(self) -> None: self._invalid_config("must be a dict") - def test_volumes_invalid_volume_type(self) -> None: + def test_invalid_list(self) -> None: """volume of incorrect type (list) raises ConfigError""" with open(".scuba.yml", "w") as f: f.write( @@ -848,7 +841,7 @@ def test_volumes_invalid_volume_type(self) -> None: self._invalid_config("must be string or dict") - def test_volumes_null_volume_type(self) -> None: + def test_null_volume_type(self) -> None: """volume of None type raises ConfigError""" # NOTE: In the future, we might want to support this as a volume # (non-bindmount, e.g. '-v /somedata'), or as tmpfs @@ -863,7 +856,7 @@ def test_volumes_null_volume_type(self) -> None: self._invalid_config("hostpath") - def test_volume_as_dict_missing_hostpath(self) -> None: + def test_complex_missing_hostpath(self) -> None: """volume of incorrect type raises ConfigError""" # NOTE: In the future, we might want to support this as a volume # (non-bindmount, e.g. '-v /somedata'), or as tmpfs @@ -879,7 +872,7 @@ def test_volume_as_dict_missing_hostpath(self) -> None: self._invalid_config("hostpath") - def test_volumes_simple_bind(self) -> None: + def test_simple_bind(self) -> None: """volumes can be set using the simple form""" with open(".scuba.yml", "w") as f: f.write( @@ -896,7 +889,7 @@ def test_volumes_simple_bind(self) -> None: assert_vol(config.volumes, "/cpath", "/hpath") - def test_volumes_complex_bind(self) -> None: + def test_complex_bind(self) -> None: """volumes can be set using the complex form""" with open(".scuba.yml", "w") as f: f.write( @@ -921,7 +914,7 @@ def test_volumes_complex_bind(self) -> None: assert_vol(vols, "/bar", "/host/bar") assert_vol(vols, "/snap", "/host/snap", ["z", "ro"]) - def test_volumes_complex_named_volume(self) -> None: + def test_complex_named_volume(self) -> None: """volumes complex form can specify a named volume""" with open(".scuba.yml", "w") as f: f.write( @@ -942,8 +935,8 @@ def test_volumes_complex_named_volume(self) -> None: assert_paths_equal(vol.container_path, "/foo") assert vol.volume_name == "foo-volume" - def test_alias_volumes_set(self) -> None: - """docker_args can be set via alias""" + def test_via_alias(self) -> None: + """volumes can be set via alias""" with open(".scuba.yml", "w") as f: f.write( r""" @@ -968,7 +961,7 @@ def test_alias_volumes_set(self) -> None: assert_vol(vols, "/foo", "/host/foo") assert_vol(vols, "/bar", "/host/bar", ["z", "ro"]) - def test_volumes_with_env_vars_simple(self, monkeypatch) -> None: + def test_with_env_vars_simple(self, monkeypatch) -> None: """volume definitions can contain environment variables""" monkeypatch.setenv("TEST_VOL_PATH", "/bar/baz") monkeypatch.setenv("TEST_VOL_PATH2", "/moo/doo") @@ -988,7 +981,7 @@ def test_volumes_with_env_vars_simple(self, monkeypatch) -> None: assert_vol(vols, "/bar/baz/foo", "/moo/doo/foo") - def test_volumes_with_env_vars_complex(self, monkeypatch) -> None: + def test_with_env_vars_complex(self, monkeypatch) -> None: """complex volume definitions can contain environment variables""" monkeypatch.setenv("TEST_HOME", "/home/testuser") monkeypatch.setenv("TEST_TMP", "/tmp") @@ -1019,7 +1012,7 @@ def test_volumes_with_env_vars_complex(self, monkeypatch) -> None: vols, "/var/spool/mail/container", "/var/spool/mail/testuser", ["z", "ro"] ) - def test_volumes_with_invalid_env_vars(self, monkeypatch) -> None: + def test_with_invalid_env_vars(self, monkeypatch) -> None: """Volume definitions cannot include unset env vars""" # Ensure that the entry does not exist in the environment monkeypatch.delenv("TEST_VAR1", raising=False) @@ -1033,7 +1026,7 @@ def test_volumes_with_invalid_env_vars(self, monkeypatch) -> None: ) self._invalid_config("TEST_VAR1") - def test_volumes_hostpath_rel(self, monkeypatch, in_tmp_path) -> None: + def test_hostpath_rel(self, monkeypatch, in_tmp_path) -> None: """volume hostpath can be relative to scuba_root (top dir)""" monkeypatch.setenv("RELVAR", "./spam/eggs") @@ -1064,7 +1057,7 @@ def test_volumes_hostpath_rel(self, monkeypatch, in_tmp_path) -> None: assert_vol(config.volumes, "/scp", in_tmp_path / "snap" / "crackle" / "pop") assert_vol(config.volumes, "/relvar", in_tmp_path / "spam" / "eggs") - def test_volumes_hostpath_rel_above(self, monkeypatch, in_tmp_path) -> None: + def test_hostpath_rel_above(self, monkeypatch, in_tmp_path) -> None: """volume hostpath can be relative, above scuba_root (top dir)""" # Directory structure: # @@ -1098,9 +1091,7 @@ def test_volumes_hostpath_rel_above(self, monkeypatch, in_tmp_path) -> None: assert config.volumes is not None assert_vol(config.volumes, "/foo", in_tmp_path / "foo_up_here") - def test_volumes_hostpath_rel_requires_dot_complex( - self, monkeypatch, in_tmp_path - ) -> None: + def test_hostpath_rel_requires_dot_complex(self, monkeypatch, in_tmp_path) -> None: """relaitve volume hostpath (complex form) must start with ./ or ../""" with open(".scuba.yml", "w") as f: f.write( @@ -1113,7 +1104,7 @@ def test_volumes_hostpath_rel_requires_dot_complex( ) self._invalid_config("Relative path must start with ./ or ../") - def test_volumes_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: + def test_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: """volume definitions can contain environment variables, including relative path portions""" monkeypatch.setenv("PREFIX", "./") with open(".scuba.yml", "w") as f: @@ -1132,7 +1123,7 @@ def test_volumes_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: assert_vol(vols, "/foo", in_tmp_path / "foo") - def test_volumes_contpath_rel(self, monkeypatch, in_tmp_path) -> None: + def test_contpath_rel(self, monkeypatch, in_tmp_path) -> None: with open(".scuba.yml", "w") as f: f.write( r""" @@ -1143,7 +1134,7 @@ def test_volumes_contpath_rel(self, monkeypatch, in_tmp_path) -> None: ) self._invalid_config("Relative path not allowed: foo") - def test_volumes_simple_named_volume(self) -> None: + def test_simple_named_volume(self) -> None: """volumes simple form can specify a named volume""" with open(".scuba.yml", "w") as f: f.write( @@ -1163,7 +1154,7 @@ def test_volumes_simple_named_volume(self) -> None: assert_paths_equal(vol.container_path, "/foo") assert vol.volume_name == "foo-volume" - def test_volumes_simple_named_volume_env(self, monkeypatch) -> None: + def test_simple_named_volume_env(self, monkeypatch) -> None: """volumes simple form can specify a named volume via env var""" with open(".scuba.yml", "w") as f: f.write( @@ -1185,7 +1176,7 @@ def test_volumes_simple_named_volume_env(self, monkeypatch) -> None: assert_paths_equal(vol.container_path, "/foo") assert vol.volume_name == "foo-volume" - def test_volumes_complex_named_volume_env(self, monkeypatch) -> None: + def test_complex_named_volume_env(self, monkeypatch) -> None: """volumes complex form can specify a named volume via env var""" with open(".scuba.yml", "w") as f: f.write( @@ -1208,7 +1199,7 @@ def test_volumes_complex_named_volume_env(self, monkeypatch) -> None: assert_paths_equal(vol.container_path, "/foo") assert vol.volume_name == "foo-volume" - def test_volumes_complex_named_volume_env_unset(self) -> None: + def test_complex_named_volume_env_unset(self) -> None: """volumes complex form fails on unset env var""" with open(".scuba.yml", "w") as f: f.write( @@ -1221,7 +1212,7 @@ def test_volumes_complex_named_volume_env_unset(self) -> None: ) self._invalid_config("Unset environment variable") - def test_volumes_complex_invalid_hostpath(self) -> None: + def test_complex_invalid_hostpath(self) -> None: """volumes complex form cannot specify an invalid hostpath""" with open(".scuba.yml", "w") as f: f.write( @@ -1234,7 +1225,7 @@ def test_volumes_complex_invalid_hostpath(self) -> None: ) self._invalid_config("Relative path must start with ./ or ../") - def test_volumes_complex_hostpath_and_name(self) -> None: + def test_complex_hostpath_and_name(self) -> None: """volumes complex form cannot specify a named volume and hostpath""" with open(".scuba.yml", "w") as f: f.write( @@ -1250,7 +1241,7 @@ def test_volumes_complex_hostpath_and_name(self) -> None: "Volume /foo must have exactly one of 'hostpath' or 'name' subkey" ) - def test_volumes_complex_empty(self) -> None: + def test_complex_empty(self) -> None: """volumes complex form cannot be empty""" with open(".scuba.yml", "w") as f: f.write( From 67f5efcb5cd5f905ea7cac15d55f23ebae15096c Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 15:59:22 -0400 Subject: [PATCH 02/25] tests: Add main tests subclasses --- tests/test_main.py | 68 ++++++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 7b242fb..f03ff83 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -29,7 +29,7 @@ @pytest.mark.usefixtures("in_tmp_path") -class TestMainBase: +class MainTest: def run_scuba(self, args, exp_retval=0, mock_isatty=False, stdin=None): """Run scuba, checking its return value @@ -88,7 +88,7 @@ def run_scuba(self, args, exp_retval=0, mock_isatty=False, stdin=None): sys.stderr = old_stderr -class TestMain(TestMainBase): +class TestMainBasic(MainTest): def test_basic(self) -> None: """Verify basic scuba functionality""" @@ -178,7 +178,7 @@ def test_no_docker(self) -> None: args = ["/bin/echo", "-n", "my output"] old_PATH = os.environ["PATH"] - os.environ["PATH"] = "" + os.environ["PATH"] = "" # TODO: Use monkeypatch try: _, err = self.run_scuba(args, 2) @@ -231,6 +231,8 @@ def test_created_file_ownership(self) -> None: assert st.st_uid == os.getuid() assert st.st_gid == os.getgid() + +class TestMainStdinStdout(MainTest): def _setup_test_tty(self) -> None: with open(".scuba.yml", "w") as f: f.write(f"image: {DOCKER_IMAGE}\n") @@ -271,6 +273,8 @@ def test_redirect_stdin(self) -> None: assert_str_equalish(out, test_str) + +class TestMainUser(MainTest): def _test_user( self, expected_uid, @@ -375,6 +379,8 @@ def test_user_root_alias(self) -> None: assert int(gid) == os.getgid() assert groupname == getgrgid(os.getgid()).gr_name + +class TestMainHomedir(MainTest): def _test_home_writable(self, scuba_args=[]): with open(".scuba.yml", "w") as f: f.write(f"image: {DOCKER_IMAGE}\n") @@ -409,6 +415,8 @@ def test_home_writable_root(self) -> None: """Verify root has a writable homedir""" self._test_home_writable(["-r"]) + +class TestMainDockerArgs(MainTest): def test_arbitrary_docker_args(self) -> None: """Verify -d successfully passes arbitrary docker arguments""" @@ -457,6 +465,24 @@ def mount_dummy(name): files = set(out.splitlines()) assert files == expfiles + +class TestMainAliasScripts(MainTest): + def test_complex_commands_in_alias(self) -> None: + """Verify complex commands can be used in alias scripts""" + test_string = "Hello world" + os.mkdir("foo") + with open("foo/bar.txt", "w") as f: + f.write(test_string) + with open(".scuba.yml", "w") as f: + f.write(f"image: {DOCKER_IMAGE}\n") + f.write("aliases:\n") + f.write(" alias1:\n") + f.write(" script:\n") + f.write(" - cd foo && cat bar.txt\n") + + out, _ = self.run_scuba(["alias1"]) + assert_str_equalish(test_string, out) + def test_nested_sript(self) -> None: """Verify nested scripts works""" with open(".scuba.yml", "w") as f: @@ -477,9 +503,8 @@ def test_nested_sript(self) -> None: out = out.replace("\n", " ") assert_str_equalish(out, test_str) - ############################################################################ - # Entrypoint +class TestMainEntrypoint(MainTest): def test_image_entrypoint(self) -> None: """Verify scuba doesn't interfere with the configured image ENTRYPOINT""" @@ -604,9 +629,8 @@ def test_yaml_entrypoint_override_none(self) -> None: # (because it didn't run) assert_str_equalish("", out) - ############################################################################ - # Image override +class TestMainImageOverride(MainTest): def test_image_override(self) -> None: """Verify --image works""" @@ -663,25 +687,8 @@ def test_yml_not_needed_with_image_override(self) -> None: out, _ = self.run_scuba(args) assert_str_equalish("success", out) - def test_complex_commands_in_alias(self) -> None: - """Verify complex commands can be used in alias scripts""" - test_string = "Hello world" - os.mkdir("foo") - with open("foo/bar.txt", "w") as f: - f.write(test_string) - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") - f.write("aliases:\n") - f.write(" alias1:\n") - f.write(" script:\n") - f.write(" - cd foo && cat bar.txt\n") - - out, _ = self.run_scuba(["alias1"]) - assert_str_equalish(test_string, out) - - ############################################################################ - # Hooks +class TestMainHooks(MainTest): def _test_one_hook(self, hookname, hookcmd, cmd, exp_retval=0): with open(".scuba.yml", "w") as f: f.write(f"image: {DOCKER_IMAGE}\n") @@ -723,9 +730,8 @@ def test_hook_failure_shows_correct_status(self) -> None: ) assert re.match(f"^scubainit: .* exited with status {testval}$", err) - ############################################################################ - # Environment +class TestMainEnvironment(MainTest): def test_env_var_keyval(self) -> None: """Verify -e KEY=VAL works""" with open(".scuba.yml", "w") as f: @@ -819,9 +825,8 @@ def test_builtin_env__SCUBA_ROOT(self, in_tmp_path): assert_str_equalish(in_tmp_path, out) - ############################################################################ - # Shell Override +class TestMainShellOverride(MainTest): def test_use_top_level_shell_override(self) -> None: """Verify that the shell can be overriden at the top level""" with open(".scuba.yml", "w") as f: @@ -923,9 +928,8 @@ def test_shell_override_precedence(self) -> None: out, _ = self.run_scuba(["--shell", "/bin/bash", "shell_check"]) assert_str_equalish("/bin/bash", out) - ############################################################################ - # Volumes +class TestMainVolumes(MainTest): def test_volumes_basic(self) -> None: """Verify volumes can be added at top-level and alias""" @@ -1137,7 +1141,7 @@ def test_volumes_hostpath_rel_above(self) -> None: assert out == test_message -class TestMainNamedVolumes(TestMainBase): +class TestMainNamedVolumes(MainTest): VOLUME_NAME = "foo-volume" def _rm_volume(self) -> None: From 9baf0e69f1445ef6da06182bf79996db5f2b88f4 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 16:16:09 -0400 Subject: [PATCH 03/25] tests: Require expect_return as keyword-only --- tests/test_main.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index f03ff83..001f778 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -30,7 +30,7 @@ @pytest.mark.usefixtures("in_tmp_path") class MainTest: - def run_scuba(self, args, exp_retval=0, mock_isatty=False, stdin=None): + def run_scuba(self, args, *, expect_return=0, mock_isatty=False, stdin=None): """Run scuba, checking its return value Returns scuba/docker stdout data. @@ -78,7 +78,7 @@ def run_scuba(self, args, exp_retval=0, mock_isatty=False, stdin=None): logging.info("scuba stderr:\n" + stderr_data) # Verify the return value was as expected - assert exp_retval == retcode + assert expect_return == retcode return stdout_data, stderr_data @@ -116,7 +116,7 @@ def test_no_image_cmd(self) -> None: f.write("image: scuba/scratch\n") # ScubaError -> exit(128) - out, _ = self.run_scuba([], 128) + out, _ = self.run_scuba([], expect_return=128) def test_handle_get_image_command_error(self) -> None: """Verify scuba handles a get_image_command error""" @@ -131,7 +131,7 @@ def mocked_gic(*args, **kw): # http://www.voidspace.org.uk/python/mock/patch.html#where-to-patch with mock.patch("scuba.scuba.get_image_command", side_effect=mocked_gic): # DockerError -> exit(128) - self.run_scuba([], 128) + self.run_scuba([], expect_return=128) def test_config_error(self) -> None: """Verify config errors are handled gracefully""" @@ -140,7 +140,7 @@ def test_config_error(self) -> None: f.write("invalid_key: is no good\n") # ConfigError -> exit(128) - self.run_scuba([], 128) + self.run_scuba([], expect_return=128) def test_multiline_alias_no_args_error(self) -> None: """Verify config errors from passing arguments to multi-line alias are caught""" @@ -158,7 +158,7 @@ def test_multiline_alias_no_args_error(self) -> None: ) # ConfigError -> exit(128) - self.run_scuba(["multi", "with", "args"], 128) + self.run_scuba(["multi", "with", "args"], expect_return=128) def test_version(self) -> None: """Verify scuba prints its version for -v""" @@ -181,7 +181,7 @@ def test_no_docker(self) -> None: os.environ["PATH"] = "" # TODO: Use monkeypatch try: - _, err = self.run_scuba(args, 2) + _, err = self.run_scuba(args, expect_return=2) finally: os.environ["PATH"] = old_PATH @@ -689,14 +689,14 @@ def test_yml_not_needed_with_image_override(self) -> None: class TestMainHooks(MainTest): - def _test_one_hook(self, hookname, hookcmd, cmd, exp_retval=0): + def _test_one_hook(self, hookname, hookcmd, cmd, expect_return=0): with open(".scuba.yml", "w") as f: f.write(f"image: {DOCKER_IMAGE}\n") f.write("hooks:\n") f.write(f" {hookname}: {hookcmd}\n") args = ["/bin/sh", "-c", cmd] - return self.run_scuba(args, exp_retval=exp_retval) + return self.run_scuba(args, expect_return=expect_return) def _test_hook_runs_as(self, hookname, exp_uid, exp_gid) -> None: out, _ = self._test_one_hook( @@ -726,7 +726,7 @@ def test_hook_failure_shows_correct_status(self) -> None: "root", f"exit {testval}", "dont care", - exp_retval=SCUBAINIT_EXIT_FAIL, + expect_return=SCUBAINIT_EXIT_FAIL, ) assert re.match(f"^scubainit: .* exited with status {testval}$", err) @@ -1074,7 +1074,7 @@ def test_volumes_host_path_failure(self) -> None: """ ) - self.run_scuba(["touch", "/userdir/test.txt"], 128) + self.run_scuba(["touch", "/userdir/test.txt"], expect_return=128) def test_volumes_host_path_rel(self) -> None: """Volume host paths can be relative""" From fcf2c09a1c01538392b2505ceb727b47de7c05e4 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 16:21:20 -0400 Subject: [PATCH 04/25] tests: Move run_scuba out of MainTest --- tests/test_main.py | 209 +++++++++++++++++++++++---------------------- 1 file changed, 106 insertions(+), 103 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 001f778..fd1a949 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -28,64 +28,67 @@ SCUBAINIT_EXIT_FAIL = 99 -@pytest.mark.usefixtures("in_tmp_path") -class MainTest: - def run_scuba(self, args, *, expect_return=0, mock_isatty=False, stdin=None): - """Run scuba, checking its return value - - Returns scuba/docker stdout data. - """ - - # Capture both scuba and docker's stdout/stderr, - # just as the user would see it. - with TemporaryFile(prefix="scubatest-stdout", mode="w+t") as stdout: - with TemporaryFile(prefix="scubatest-stderr", mode="w+t") as stderr: - if mock_isatty: - stdout = PseudoTTY(stdout) - stderr = PseudoTTY(stderr) - - old_stdin = sys.stdin - old_stdout = sys.stdout - old_stderr = sys.stderr - - if stdin is None: - sys.stdin = open(os.devnull, "w") +def run_scuba(args, *, expect_return=0, mock_isatty=False, stdin=None): + """Run scuba, checking its return value + + Returns scuba/docker stdout data. + """ + + # Capture both scuba and docker's stdout/stderr, + # just as the user would see it. + with TemporaryFile(prefix="scubatest-stdout", mode="w+t") as stdout: + with TemporaryFile(prefix="scubatest-stderr", mode="w+t") as stderr: + if mock_isatty: + stdout = PseudoTTY(stdout) + stderr = PseudoTTY(stderr) + + old_stdin = sys.stdin + old_stdout = sys.stdout + old_stderr = sys.stderr + + if stdin is None: + sys.stdin = open(os.devnull, "w") + else: + sys.stdin = stdin + sys.stdout = stdout + sys.stderr = stderr + + try: + """ + Call scuba's main(), and expect it to either exit() + with a given return code, or return (implying an exit + status of 0). + """ + try: + main.main(argv=args) + except SystemExit as sysexit: + retcode = sysexit.code else: - sys.stdin = stdin - sys.stdout = stdout - sys.stderr = stderr + retcode = 0 - try: - """ - Call scuba's main(), and expect it to either exit() - with a given return code, or return (implying an exit - status of 0). - """ - try: - main.main(argv=args) - except SystemExit as sysexit: - retcode = sysexit.code - else: - retcode = 0 + stdout.seek(0) + stderr.seek(0) + + stdout_data = stdout.read() + stderr_data = stderr.read() - stdout.seek(0) - stderr.seek(0) + logging.info("scuba stdout:\n" + stdout_data) + logging.info("scuba stderr:\n" + stderr_data) - stdout_data = stdout.read() - stderr_data = stderr.read() + # Verify the return value was as expected + assert expect_return == retcode - logging.info("scuba stdout:\n" + stdout_data) - logging.info("scuba stderr:\n" + stderr_data) + return stdout_data, stderr_data - # Verify the return value was as expected - assert expect_return == retcode + finally: + sys.stdin = old_stdin + sys.stdout = old_stdout + sys.stderr = old_stderr - return stdout_data, stderr_data - finally: - sys.stdin = old_stdin - sys.stdout = old_stdout - sys.stderr = old_stderr +@pytest.mark.usefixtures("in_tmp_path") +class MainTest: + pass class TestMainBasic(MainTest): @@ -96,7 +99,7 @@ def test_basic(self) -> None: f.write(f"image: {DOCKER_IMAGE}\n") args = ["/bin/echo", "-n", "my output"] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish("my output", out) @@ -106,7 +109,7 @@ def test_no_cmd(self) -> None: with open(".scuba.yml", "w") as f: f.write("image: scuba/hello\n") - out, _ = self.run_scuba([]) + out, _ = run_scuba([]) assert_str_equalish(out, "Hello world") def test_no_image_cmd(self) -> None: @@ -116,7 +119,7 @@ def test_no_image_cmd(self) -> None: f.write("image: scuba/scratch\n") # ScubaError -> exit(128) - out, _ = self.run_scuba([], expect_return=128) + out, _ = run_scuba([], expect_return=128) def test_handle_get_image_command_error(self) -> None: """Verify scuba handles a get_image_command error""" @@ -131,7 +134,7 @@ def mocked_gic(*args, **kw): # http://www.voidspace.org.uk/python/mock/patch.html#where-to-patch with mock.patch("scuba.scuba.get_image_command", side_effect=mocked_gic): # DockerError -> exit(128) - self.run_scuba([], expect_return=128) + run_scuba([], expect_return=128) def test_config_error(self) -> None: """Verify config errors are handled gracefully""" @@ -140,7 +143,7 @@ def test_config_error(self) -> None: f.write("invalid_key: is no good\n") # ConfigError -> exit(128) - self.run_scuba([], expect_return=128) + run_scuba([], expect_return=128) def test_multiline_alias_no_args_error(self) -> None: """Verify config errors from passing arguments to multi-line alias are caught""" @@ -158,12 +161,12 @@ def test_multiline_alias_no_args_error(self) -> None: ) # ConfigError -> exit(128) - self.run_scuba(["multi", "with", "args"], expect_return=128) + run_scuba(["multi", "with", "args"], expect_return=128) def test_version(self) -> None: """Verify scuba prints its version for -v""" - out, err = self.run_scuba(["-v"]) + out, err = run_scuba(["-v"]) name, ver = out.split() assert name == "scuba" @@ -181,7 +184,7 @@ def test_no_docker(self) -> None: os.environ["PATH"] = "" # TODO: Use monkeypatch try: - _, err = self.run_scuba(args, expect_return=2) + _, err = run_scuba(args, expect_return=2) finally: os.environ["PATH"] = old_PATH @@ -194,7 +197,7 @@ def test_dry_run(self, subproc_call_mock): args = ["--dry-run", "--verbose", "/bin/false"] - _, err = self.run_scuba(args) + _, err = run_scuba(args) assert not subproc_call_mock.called @@ -213,7 +216,7 @@ def test_args(self) -> None: lines = ["here", "are", "some args"] - out, _ = self.run_scuba(["./test.sh"] + lines) + out, _ = run_scuba(["./test.sh"] + lines) assert_seq_equal(out.splitlines(), lines) @@ -225,7 +228,7 @@ def test_created_file_ownership(self) -> None: filename = "newfile.txt" - self.run_scuba(["/bin/touch", filename]) + run_scuba(["/bin/touch", filename]) st = os.stat(filename) assert st.st_uid == os.getuid() @@ -247,7 +250,7 @@ def test_with_tty(self) -> None: """Verify docker allocates tty if stdout is a tty.""" self._setup_test_tty() - out, _ = self.run_scuba(["./check_tty.sh"], mock_isatty=True) + out, _ = run_scuba(["./check_tty.sh"], mock_isatty=True) assert_str_equalish(out, "isatty") @@ -256,7 +259,7 @@ def test_without_tty(self) -> None: """Verify docker doesn't allocate tty if stdout is not a tty.""" self._setup_test_tty() - out, _ = self.run_scuba(["./check_tty.sh"]) + out, _ = run_scuba(["./check_tty.sh"]) assert_str_equalish(out, "notatty") @@ -269,7 +272,7 @@ def test_redirect_stdin(self) -> None: with TemporaryFile(prefix="scubatest-stdin", mode="w+t") as stdin: stdin.write(test_str) stdin.seek(0) - out, _ = self.run_scuba(["cat"], stdin=stdin) + out, _ = run_scuba(["cat"], stdin=stdin) assert_str_equalish(out, test_str) @@ -291,7 +294,7 @@ def _test_user( "-c", "echo $(id -u) $(id -un) $(id -g) $(id -gn)", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) uid, username, gid, groupname = out.split() uid = int(uid) @@ -349,7 +352,7 @@ def test_user_root_alias(self) -> None: """ ) - out, _ = self.run_scuba(["root_test"]) + out, _ = run_scuba(["root_test"]) uid, username, gid, groupname = out.split() assert int(uid) == 0 @@ -371,7 +374,7 @@ def test_user_root_alias(self) -> None: """ ) - out, _ = self.run_scuba(["no_root_test"]) + out, _ = run_scuba(["no_root_test"]) uid, username, gid, groupname = out.split() assert int(uid) == os.getuid() @@ -390,7 +393,7 @@ def _test_home_writable(self, scuba_args=[]): "-c", "echo success >> ~/testfile; cat ~/testfile", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish(out, "success") @@ -435,7 +438,7 @@ def test_arbitrary_docker_args(self) -> None: "cat", data_path, ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish(out, data) @@ -460,7 +463,7 @@ def mount_dummy(name): "ls", tgtdir, ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) files = set(out.splitlines()) assert files == expfiles @@ -480,7 +483,7 @@ def test_complex_commands_in_alias(self) -> None: f.write(" script:\n") f.write(" - cd foo && cat bar.txt\n") - out, _ = self.run_scuba(["alias1"]) + out, _ = run_scuba(["alias1"]) assert_str_equalish(test_string, out) def test_nested_sript(self) -> None: @@ -498,7 +501,7 @@ def test_nested_sript(self) -> None: f.write(' - echo "crazy"\n') test_str = "This list is nested kinda crazy" - out, _ = self.run_scuba(["foo"]) + out, _ = run_scuba(["foo"]) out = out.replace("\n", " ") assert_str_equalish(out, test_str) @@ -511,7 +514,7 @@ def test_image_entrypoint(self) -> None: with open(".scuba.yml", "w") as f: f.write("image: scuba/entrypoint-test") - out, _ = self.run_scuba(["cat", "entrypoint_works.txt"]) + out, _ = run_scuba(["cat", "entrypoint_works.txt"]) assert_str_equalish("success", out) def test_image_entrypoint_multiline(self) -> None: @@ -528,7 +531,7 @@ def test_image_entrypoint_multiline(self) -> None: """ ) - out, _ = self.run_scuba(["testalias"]) + out, _ = run_scuba(["testalias"]) assert_str_equalish("\n".join(["success"] * 2), out) def test_entrypoint_override(self) -> None: @@ -556,7 +559,7 @@ def test_entrypoint_override(self) -> None: os.path.abspath("new.sh"), "true", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish(test_str, out) def test_entrypoint_override_none(self) -> None: @@ -577,7 +580,7 @@ def test_entrypoint_override_none(self) -> None: "", "testalias", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) # Verify that ENTRYPOINT_WORKS was not set by the entrypoint # (because it didn't run) @@ -603,7 +606,7 @@ def test_yaml_entrypoint_override(self) -> None: args = [ "true", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish(test_str, out) def test_yaml_entrypoint_override_none(self) -> None: @@ -623,7 +626,7 @@ def test_yaml_entrypoint_override_none(self) -> None: args = [ "testalias", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) # Verify that ENTRYPOINT_WORKS was not set by the entrypoint # (because it didn't run) @@ -644,7 +647,7 @@ def test_image_override(self) -> None: "echo", "success", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish("success", out) def test_image_override_with_alias(self) -> None: @@ -670,7 +673,7 @@ def test_image_override_with_alias(self) -> None: DOCKER_IMAGE, "testalias", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish("multi\nline\nalias", out) def test_yml_not_needed_with_image_override(self) -> None: @@ -684,7 +687,7 @@ def test_yml_not_needed_with_image_override(self) -> None: "echo", "success", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish("success", out) @@ -696,7 +699,7 @@ def _test_one_hook(self, hookname, hookcmd, cmd, expect_return=0): f.write(f" {hookname}: {hookcmd}\n") args = ["/bin/sh", "-c", cmd] - return self.run_scuba(args, expect_return=expect_return) + return run_scuba(args, expect_return=expect_return) def _test_hook_runs_as(self, hookname, exp_uid, exp_gid) -> None: out, _ = self._test_one_hook( @@ -743,7 +746,7 @@ def test_env_var_keyval(self) -> None: "-c", "echo $KEY", ] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish(out, "VAL") def test_env_var_key_only(self, monkeypatch): @@ -758,7 +761,7 @@ def test_env_var_key_only(self, monkeypatch): "echo $KEY", ] monkeypatch.setenv("KEY", "mockedvalue") - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish(out, "mockedvalue") def test_env_var_sources(self, monkeypatch): @@ -800,7 +803,7 @@ def test_env_var_sources(self, monkeypatch): monkeypatch.setenv("EXTERNAL_2", "External value 2") monkeypatch.setenv("EXTERNAL_3", "External value 3") - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) # Convert key/pair output to dictionary result = dict(pair.split("=", 1) for pair in shlex.split(out)) @@ -821,7 +824,7 @@ def test_builtin_env__SCUBA_ROOT(self, in_tmp_path): f.write(f"image: {DOCKER_IMAGE}\n") args = ["/bin/sh", "-c", "echo $SCUBA_ROOT"] - out, _ = self.run_scuba(args) + out, _ = run_scuba(args) assert_str_equalish(in_tmp_path, out) @@ -840,7 +843,7 @@ def test_use_top_level_shell_override(self) -> None: """ ) - out, _ = self.run_scuba(["check_shell"]) + out, _ = run_scuba(["check_shell"]) # If we failed to override, the shebang would be #!/bin/sh assert_str_equalish("/bin/bash", out) @@ -858,10 +861,10 @@ def test_alias_level_shell_override(self) -> None: script: readlink -f /proc/$$/exe """ ) - out, _ = self.run_scuba(["shell_override"]) + out, _ = run_scuba(["shell_override"]) assert_str_equalish("/bin/bash", out) - out, _ = self.run_scuba(["default_shell"]) + out, _ = run_scuba(["default_shell"]) # The way that we check the shell uses the resolved symlink of /bin/sh, # which is /bin/dash on Debian assert out.strip() in ["/bin/sh", "/bin/dash"] @@ -878,7 +881,7 @@ def test_cli_shell_override(self) -> None: """ ) - out, _ = self.run_scuba(["--shell", "/bin/bash", "default_shell"]) + out, _ = run_scuba(["--shell", "/bin/bash", "default_shell"]) assert_str_equalish("/bin/bash", out) def test_shell_override_precedence(self) -> None: @@ -898,7 +901,7 @@ def test_shell_override_precedence(self) -> None: script: readlink -f /proc/$$/exe """ ) - out, _ = self.run_scuba(["shell_override"]) + out, _ = run_scuba(["shell_override"]) assert_str_equalish("/bin/bash", out) # Test alias-level << CLI @@ -912,7 +915,7 @@ def test_shell_override_precedence(self) -> None: script: readlink -f /proc/$$/exe """ ) - out, _ = self.run_scuba(["--shell", "/bin/bash", "shell_overridden"]) + out, _ = run_scuba(["--shell", "/bin/bash", "shell_overridden"]) assert_str_equalish("/bin/bash", out) # Test top-level << CLI @@ -925,7 +928,7 @@ def test_shell_override_precedence(self) -> None: shell_check: readlink -f /proc/$$/exe """ ) - out, _ = self.run_scuba(["--shell", "/bin/bash", "shell_check"]) + out, _ = run_scuba(["--shell", "/bin/bash", "shell_check"]) assert_str_equalish("/bin/bash", out) @@ -956,7 +959,7 @@ def test_volumes_basic(self) -> None: """ ) - out, _ = self.run_scuba(["doit"]) + out, _ = run_scuba(["doit"]) out = out.splitlines() assert out == ["from the top", "from the alias"] @@ -987,12 +990,12 @@ def test_volumes_alias_override(self) -> None: ) # Run a non-alias command - out, _ = self.run_scuba(["cat", "/data/thing"]) + out, _ = run_scuba(["cat", "/data/thing"]) out = out.splitlines() assert out == ["from the top"] # Run the alias - out, _ = self.run_scuba(["doit"]) + out, _ = run_scuba(["doit"]) out = out.splitlines() assert out == ["from the alias"] @@ -1011,7 +1014,7 @@ def test_volumes_host_path_create(self) -> None: """ ) - self.run_scuba(["touch", "/userdir/test.txt"]) + run_scuba(["touch", "/userdir/test.txt"]) assert testfile.exists(), "Test file was not created" @@ -1044,7 +1047,7 @@ def test_volumes_host_path_permissions(self) -> None: try: # Prevent current user from creating directory rootdir.chmod(0o555) - self.run_scuba(["doit"]) + run_scuba(["doit"]) finally: # Restore permissions to allow deletion rootdir.chmod(0o755) @@ -1074,7 +1077,7 @@ def test_volumes_host_path_failure(self) -> None: """ ) - self.run_scuba(["touch", "/userdir/test.txt"], expect_return=128) + run_scuba(["touch", "/userdir/test.txt"], expect_return=128) def test_volumes_host_path_rel(self) -> None: """Volume host paths can be relative""" @@ -1100,7 +1103,7 @@ def test_volumes_host_path_rel(self) -> None: otherdir.mkdir(parents=True) os.chdir(otherdir) - out, _ = self.run_scuba(["cat", "/userdir/test.txt"]) + out, _ = run_scuba(["cat", "/userdir/test.txt"]) assert out == test_message def test_volumes_hostpath_rel_above(self) -> None: @@ -1137,7 +1140,7 @@ def test_volumes_hostpath_rel_above(self) -> None: """ ) - out, _ = self.run_scuba(["cat", "/userdir/test.txt"]) + out, _ = run_scuba(["cat", "/userdir/test.txt"]) assert out == test_message @@ -1178,8 +1181,8 @@ def test_volumes_named(self) -> None: ) # Inoke scuba once: Write a file to the named volume - self.run_scuba(["/bin/sh", "-c", f"echo {TEST_STR} > {TEST_PATH}"]) + run_scuba(["/bin/sh", "-c", f"echo {TEST_STR} > {TEST_PATH}"]) # Invoke scuba again: Verify the file is still there - out, _ = self.run_scuba(["/bin/sh", "-c", f"cat {TEST_PATH}"]) + out, _ = run_scuba(["/bin/sh", "-c", f"cat {TEST_PATH}"]) assert_str_equalish(out, TEST_STR) From 3f12a7cf7bed3b43fa87cde0d87ee01296ef6eb2 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 16:24:09 -0400 Subject: [PATCH 05/25] tests: Use monkeypatch.setenv() instead of actual env manipulation --- tests/test_main.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index fd1a949..8accb7b 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -172,7 +172,7 @@ def test_version(self) -> None: assert name == "scuba" assert ver == scuba.__version__ - def test_no_docker(self) -> None: + def test_no_docker(self, monkeypatch) -> None: """Verify scuba gracefully handles docker not being installed""" with open(".scuba.yml", "w") as f: @@ -180,13 +180,8 @@ def test_no_docker(self) -> None: args = ["/bin/echo", "-n", "my output"] - old_PATH = os.environ["PATH"] - os.environ["PATH"] = "" # TODO: Use monkeypatch - - try: - _, err = run_scuba(args, expect_return=2) - finally: - os.environ["PATH"] = old_PATH + monkeypatch.setenv("PATH", "") + _, err = run_scuba(args, expect_return=2) @mock.patch("subprocess.call") def test_dry_run(self, subproc_call_mock): From 22f35002ec990e0f1eaac793db9ccd5d94de8edc Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 17:30:53 -0400 Subject: [PATCH 06/25] tests: Consistently write .scuba.yml using Path.write_text() --- tests/test_main.py | 631 +++++++++++++++++++++------------------------ 1 file changed, 294 insertions(+), 337 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 8accb7b..2aa5096 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -25,6 +25,7 @@ ) +SCUBA_YML = Path(".scuba.yml") SCUBAINIT_EXIT_FAIL = 99 @@ -94,9 +95,7 @@ class MainTest: class TestMainBasic(MainTest): def test_basic(self) -> None: """Verify basic scuba functionality""" - - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = ["/bin/echo", "-n", "my output"] out, _ = run_scuba(args) @@ -105,27 +104,21 @@ def test_basic(self) -> None: def test_no_cmd(self) -> None: """Verify scuba works with no given command""" - - with open(".scuba.yml", "w") as f: - f.write("image: scuba/hello\n") + SCUBA_YML.write_text("image: scuba/hello") out, _ = run_scuba([]) assert_str_equalish(out, "Hello world") def test_no_image_cmd(self) -> None: """Verify scuba gracefully handles an image with no Cmd and no user command""" - - with open(".scuba.yml", "w") as f: - f.write("image: scuba/scratch\n") + SCUBA_YML.write_text("image: scuba/scratch") # ScubaError -> exit(128) out, _ = run_scuba([], expect_return=128) def test_handle_get_image_command_error(self) -> None: """Verify scuba handles a get_image_command error""" - - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text("image: {DOCKER_IMAGE}") def mocked_gic(*args, **kw): raise scuba.dockerutil.DockerError("mock error") @@ -138,27 +131,24 @@ def mocked_gic(*args, **kw): def test_config_error(self) -> None: """Verify config errors are handled gracefully""" - - with open(".scuba.yml", "w") as f: - f.write("invalid_key: is no good\n") + SCUBA_YML.write_text("invalid_key: is no good") # ConfigError -> exit(128) run_scuba([], expect_return=128) def test_multiline_alias_no_args_error(self) -> None: """Verify config errors from passing arguments to multi-line alias are caught""" - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - aliases: - multi: - script: - - echo multi - - echo line - - echo alias - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + aliases: + multi: + script: + - echo multi + - echo line + - echo alias + """ + ) # ConfigError -> exit(128) run_scuba(["multi", "with", "args"], expect_return=128) @@ -174,9 +164,7 @@ def test_version(self) -> None: def test_no_docker(self, monkeypatch) -> None: """Verify scuba gracefully handles docker not being installed""" - - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = ["/bin/echo", "-n", "my output"] @@ -186,9 +174,7 @@ def test_no_docker(self, monkeypatch) -> None: @mock.patch("subprocess.call") def test_dry_run(self, subproc_call_mock): """Verify scuba handles --dry-run and --verbose""" - - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = ["--dry-run", "--verbose", "/bin/false"] @@ -200,11 +186,9 @@ def test_dry_run(self, subproc_call_mock): def test_args(self) -> None: """Verify scuba handles cmdline args""" + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") - - with open("test.sh", "w") as f: + with open("test.sh", "w") as f: # TODO Path.write_text() f.write("#!/bin/sh\n") f.write('for a in "$@"; do echo $a; done\n') make_executable("test.sh") @@ -217,10 +201,7 @@ def test_args(self) -> None: def test_created_file_ownership(self) -> None: """Verify files created under scuba have correct ownership""" - - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") - + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") filename = "newfile.txt" run_scuba(["/bin/touch", filename]) @@ -232,9 +213,7 @@ def test_created_file_ownership(self) -> None: class TestMainStdinStdout(MainTest): def _setup_test_tty(self) -> None: - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") - + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") with open("check_tty.sh", "w") as f: f.write("#!/bin/sh\n") f.write('if [ -t 1 ]; then echo "isatty"; else echo "notatty"; fi\n') @@ -260,8 +239,7 @@ def test_without_tty(self) -> None: def test_redirect_stdin(self) -> None: """Verify stdin redirection works""" - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") test_str = "hello world" with TemporaryFile(prefix="scubatest-stdin", mode="w+t") as stdin: @@ -281,8 +259,7 @@ def _test_user( expected_groupname, scuba_args=[], ): - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = scuba_args + [ "/bin/sh", @@ -335,17 +312,16 @@ def test_user_run_as_root(self) -> None: def test_user_root_alias(self) -> None: """Verify that aliases can set whether the container is run as root""" - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - aliases: - root_test: - root: true - script: - - echo $(id -u) $(id -un) $(id -g) $(id -gn) - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + aliases: + root_test: + root: true + script: + - echo $(id -u) $(id -un) $(id -g) $(id -gn) + """ + ) out, _ = run_scuba(["root_test"]) uid, username, gid, groupname = out.split() @@ -357,17 +333,16 @@ def test_user_root_alias(self) -> None: # No one should ever specify 'root: false' in an alias, but Scuba should behave # correctly if they do - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - aliases: - no_root_test: - root: false - script: - - echo $(id -u) $(id -un) $(id -g) $(id -gn) - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + aliases: + no_root_test: + root: false + script: + - echo $(id -u) $(id -un) $(id -g) $(id -gn) + """ + ) out, _ = run_scuba(["no_root_test"]) uid, username, gid, groupname = out.split() @@ -380,8 +355,7 @@ def test_user_root_alias(self) -> None: class TestMainHomedir(MainTest): def _test_home_writable(self, scuba_args=[]): - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = scuba_args + [ "/bin/sh", @@ -417,9 +391,7 @@ def test_home_writable_root(self) -> None: class TestMainDockerArgs(MainTest): def test_arbitrary_docker_args(self) -> None: """Verify -d successfully passes arbitrary docker arguments""" - - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") data = "Lorem ipsum dolor sit amet" data_path = "/lorem/ipsum" @@ -449,9 +421,12 @@ def mount_dummy(name): expfiles.add(name) return f'-v "{dummy.absolute()}:{tgtdir}/{name}"\n' - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") - f.write("docker_args: " + mount_dummy("one")) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + docker_args: {mount_dummy('one')} + """ + ) args = [ "-d=" + mount_dummy("two"), @@ -471,29 +446,36 @@ def test_complex_commands_in_alias(self) -> None: os.mkdir("foo") with open("foo/bar.txt", "w") as f: f.write(test_string) - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") - f.write("aliases:\n") - f.write(" alias1:\n") - f.write(" script:\n") - f.write(" - cd foo && cat bar.txt\n") + + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + aliases: + alias1: + script: + - cd foo && cat bar.txt + """ + ) out, _ = run_scuba(["alias1"]) assert_str_equalish(test_string, out) def test_nested_sript(self) -> None: """Verify nested scripts works""" - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") - f.write("aliases:\n") - f.write(" foo:\n") - f.write(" script:\n") - f.write(' - echo "This"\n') - f.write(' - - echo "list"\n') - f.write(' - echo "is"\n') - f.write(' - echo "nested"\n') - f.write(' - - echo "kinda"\n') - f.write(' - echo "crazy"\n') + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + aliases: + foo: + script: + - echo "This" + - - echo "list" + - echo "is" + - echo "nested" + - - echo "kinda" + - echo "crazy" + """ + ) test_str = "This list is nested kinda crazy" out, _ = run_scuba(["foo"]) @@ -505,42 +487,38 @@ def test_nested_sript(self) -> None: class TestMainEntrypoint(MainTest): def test_image_entrypoint(self) -> None: """Verify scuba doesn't interfere with the configured image ENTRYPOINT""" - - with open(".scuba.yml", "w") as f: - f.write("image: scuba/entrypoint-test") + SCUBA_YML.write_text("image: scuba/entrypoint-test") out, _ = run_scuba(["cat", "entrypoint_works.txt"]) assert_str_equalish("success", out) def test_image_entrypoint_multiline(self) -> None: """Verify entrypoints are handled correctly with multi-line scripts""" - with open(".scuba.yml", "w") as f: - f.write( - """ - image: scuba/entrypoint-test - aliases: - testalias: - script: - - cat entrypoint_works.txt - - echo $ENTRYPOINT_WORKS - """ - ) + SCUBA_YML.write_text( + """ + image: scuba/entrypoint-test + aliases: + testalias: + script: + - cat entrypoint_works.txt + - echo $ENTRYPOINT_WORKS + """ + ) out, _ = run_scuba(["testalias"]) assert_str_equalish("\n".join(["success"] * 2), out) def test_entrypoint_override(self) -> None: """Verify --entrypoint override works""" - with open(".scuba.yml", "w") as f: - f.write( - """ - image: scuba/entrypoint-test - aliases: - testalias: - script: - - echo $ENTRYPOINT_WORKS - """ - ) + SCUBA_YML.write_text( + """ + image: scuba/entrypoint-test + aliases: + testalias: + script: + - echo $ENTRYPOINT_WORKS + """ + ) test_str = "This is output from the overridden entrypoint" @@ -559,16 +537,15 @@ def test_entrypoint_override(self) -> None: def test_entrypoint_override_none(self) -> None: """Verify --entrypoint override (to nothing) works""" - with open(".scuba.yml", "w") as f: - f.write( - """ - image: scuba/entrypoint-test - aliases: - testalias: - script: - - echo $ENTRYPOINT_WORKS - """ - ) + SCUBA_YML.write_text( + """ + image: scuba/entrypoint-test + aliases: + testalias: + script: + - echo $ENTRYPOINT_WORKS + """ + ) args = [ "--entrypoint", @@ -583,13 +560,12 @@ def test_entrypoint_override_none(self) -> None: def test_yaml_entrypoint_override(self) -> None: """Verify entrypoint in .scuba.yml works""" - with open(".scuba.yml", "w") as f: - f.write( - """ - image: scuba/entrypoint-test - entrypoint: "./new.sh" - """ - ) + SCUBA_YML.write_text( + """ + image: scuba/entrypoint-test + entrypoint: "./new.sh" + """ + ) test_str = "This is output from the overridden entrypoint" @@ -606,17 +582,16 @@ def test_yaml_entrypoint_override(self) -> None: def test_yaml_entrypoint_override_none(self) -> None: """Verify "none" entrypoint in .scuba.yml works""" - with open(".scuba.yml", "w") as f: - f.write( - """ - image: scuba/entrypoint-test - entrypoint: - aliases: - testalias: - script: - - echo $ENTRYPOINT_WORKS - """ - ) + SCUBA_YML.write_text( + """ + image: scuba/entrypoint-test + entrypoint: + aliases: + testalias: + script: + - echo $ENTRYPOINT_WORKS + """ + ) args = [ "testalias", @@ -631,10 +606,10 @@ def test_yaml_entrypoint_override_none(self) -> None: class TestMainImageOverride(MainTest): def test_image_override(self) -> None: """Verify --image works""" - - with open(".scuba.yml", "w") as f: + SCUBA_YML.write_text( # This image does not exist - f.write("image: scuba/notheredoesnotexistbb7e344f9722\n") + "image: scuba/notheredoesnotexistbb7e344f9722" + ) args = [ "--image", @@ -647,21 +622,19 @@ def test_image_override(self) -> None: def test_image_override_with_alias(self) -> None: """Verify --image works with aliases""" - - with open(".scuba.yml", "w") as f: + SCUBA_YML.write_text( # These images do not exist - f.write( - """ - image: scuba/notheredoesnotexistbb7e344f9722 - aliases: - testalias: - image: scuba/notheredoesnotexist765205d09dea - script: - - echo multi - - echo line - - echo alias - """ - ) + """ + image: scuba/notheredoesnotexistbb7e344f9722 + aliases: + testalias: + image: scuba/notheredoesnotexist765205d09dea + script: + - echo multi + - echo line + - echo alias + """ + ) args = [ "--image", @@ -688,10 +661,13 @@ def test_yml_not_needed_with_image_override(self) -> None: class TestMainHooks(MainTest): def _test_one_hook(self, hookname, hookcmd, cmd, expect_return=0): - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") - f.write("hooks:\n") - f.write(f" {hookname}: {hookcmd}\n") + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + hooks: + {hookname}: {hookcmd} + """ + ) args = ["/bin/sh", "-c", cmd] return run_scuba(args, expect_return=expect_return) @@ -732,8 +708,7 @@ def test_hook_failure_shows_correct_status(self) -> None: class TestMainEnvironment(MainTest): def test_env_var_keyval(self) -> None: """Verify -e KEY=VAL works""" - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = [ "-e", "KEY=VAL", @@ -746,8 +721,7 @@ def test_env_var_keyval(self) -> None: def test_env_var_key_only(self, monkeypatch): """Verify -e KEY works""" - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = [ "-e", "KEY", @@ -761,30 +735,29 @@ def test_env_var_key_only(self, monkeypatch): def test_env_var_sources(self, monkeypatch): """Verify scuba handles all possible environment variable sources""" - with open(".scuba.yml", "w") as f: - f.write( - rf""" - image: {DOCKER_IMAGE} + SCUBA_YML.write_text( + rf""" + image: {DOCKER_IMAGE} + environment: + FOO: Top-level + BAR: 42 + EXTERNAL_2: + aliases: + al: + script: + - echo "FOO=\"$FOO\"" + - echo "BAR=\"$BAR\"" + - echo "MORE=\"$MORE\"" + - echo "EXTERNAL_1=\"$EXTERNAL_1\"" + - echo "EXTERNAL_2=\"$EXTERNAL_2\"" + - echo "EXTERNAL_3=\"$EXTERNAL_3\"" + - echo "BAZ=\"$BAZ\"" environment: - FOO: Top-level - BAR: 42 - EXTERNAL_2: - aliases: - al: - script: - - echo "FOO=\"$FOO\"" - - echo "BAR=\"$BAR\"" - - echo "MORE=\"$MORE\"" - - echo "EXTERNAL_1=\"$EXTERNAL_1\"" - - echo "EXTERNAL_2=\"$EXTERNAL_2\"" - - echo "EXTERNAL_3=\"$EXTERNAL_3\"" - - echo "BAZ=\"$BAZ\"" - environment: - FOO: Overridden - MORE: Hello world - EXTERNAL_3: - """ - ) + FOO: Overridden + MORE: Hello world + EXTERNAL_3: + """ + ) args = [ "-e", @@ -815,8 +788,7 @@ def test_env_var_sources(self, monkeypatch): def test_builtin_env__SCUBA_ROOT(self, in_tmp_path): """Verify SCUBA_ROOT is set in container""" - with open(".scuba.yml", "w") as f: - f.write(f"image: {DOCKER_IMAGE}\n") + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = ["/bin/sh", "-c", "echo $SCUBA_ROOT"] out, _ = run_scuba(args) @@ -827,16 +799,15 @@ def test_builtin_env__SCUBA_ROOT(self, in_tmp_path): class TestMainShellOverride(MainTest): def test_use_top_level_shell_override(self) -> None: """Verify that the shell can be overriden at the top level""" - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - shell: /bin/bash - aliases: - check_shell: - script: readlink -f /proc/$$/exe - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + shell: /bin/bash + aliases: + check_shell: + script: readlink -f /proc/$$/exe + """ + ) out, _ = run_scuba(["check_shell"]) # If we failed to override, the shebang would be #!/bin/sh @@ -844,18 +815,17 @@ def test_use_top_level_shell_override(self) -> None: def test_alias_level_shell_override(self) -> None: """Verify that the shell can be overriden at the alias level without affecting other aliases""" - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - aliases: - shell_override: - shell: /bin/bash - script: readlink -f /proc/$$/exe - default_shell: - script: readlink -f /proc/$$/exe - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + aliases: + shell_override: + shell: /bin/bash + script: readlink -f /proc/$$/exe + default_shell: + script: readlink -f /proc/$$/exe + """ + ) out, _ = run_scuba(["shell_override"]) assert_str_equalish("/bin/bash", out) @@ -866,16 +836,14 @@ def test_alias_level_shell_override(self) -> None: def test_cli_shell_override(self) -> None: """Verify that the shell can be overriden by the CLI""" - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - aliases: - default_shell: - script: readlink -f /proc/$$/exe - """ - ) - + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + aliases: + default_shell: + script: readlink -f /proc/$$/exe + """ + ) out, _ = run_scuba(["--shell", "/bin/bash", "default_shell"]) assert_str_equalish("/bin/bash", out) @@ -885,44 +853,41 @@ def test_shell_override_precedence(self) -> None: # Top-level SCUBA_YML shell << alias-level SCUBA_YML shell << CLI-specified shell # Test top-level << alias-level - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - shell: /bin/this_does_not_exist - aliases: - shell_override: - shell: /bin/bash - script: readlink -f /proc/$$/exe - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + shell: /bin/this_does_not_exist + aliases: + shell_override: + shell: /bin/bash + script: readlink -f /proc/$$/exe + """ + ) out, _ = run_scuba(["shell_override"]) assert_str_equalish("/bin/bash", out) # Test alias-level << CLI - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - aliases: - shell_overridden: - shell: /bin/this_is_not_a_real_shell - script: readlink -f /proc/$$/exe - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + aliases: + shell_overridden: + shell: /bin/this_is_not_a_real_shell + script: readlink -f /proc/$$/exe + """ + ) out, _ = run_scuba(["--shell", "/bin/bash", "shell_overridden"]) assert_str_equalish("/bin/bash", out) # Test top-level << CLI - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - shell: /bin/this_is_not_a_real_shell - aliases: - shell_check: readlink -f /proc/$$/exe - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + shell: /bin/this_is_not_a_real_shell + aliases: + shell_check: readlink -f /proc/$$/exe + """ + ) out, _ = run_scuba(["--shell", "/bin/bash", "shell_check"]) assert_str_equalish("/bin/bash", out) @@ -940,19 +905,18 @@ def test_volumes_basic(self) -> None: aliasdata.mkdir() (aliasdata / "thing").write_text("from the alias\n") - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + volumes: + /topdata: {topdata.absolute()} + aliases: + doit: volumes: - /topdata: {topdata.absolute()} - aliases: - doit: - volumes: - /aliasdata: {aliasdata.absolute()} - script: "cat /topdata/thing /aliasdata/thing" - """ - ) + /aliasdata: {aliasdata.absolute()} + script: "cat /topdata/thing /aliasdata/thing" + """ + ) out, _ = run_scuba(["doit"]) out = out.splitlines() @@ -970,19 +934,18 @@ def test_volumes_alias_override(self) -> None: aliasdata.mkdir() (aliasdata / "thing").write_text("from the alias\n") - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + volumes: + /data: {topdata.absolute()} + aliases: + doit: volumes: - /data: {topdata.absolute()} - aliases: - doit: - volumes: - /data: {aliasdata.absolute()} - script: "cat /data/thing" - """ - ) + /data: {aliasdata.absolute()} + script: "cat /data/thing" + """ + ) # Run a non-alias command out, _ = run_scuba(["cat", "/data/thing"]) @@ -1000,14 +963,13 @@ def test_volumes_host_path_create(self) -> None: userdir = Path("./user") testfile = userdir / "test.txt" - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - volumes: - /userdir: {userdir.absolute()} - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + volumes: + /userdir: {userdir.absolute()} + """ + ) run_scuba(["touch", "/userdir/test.txt"]) @@ -1026,18 +988,17 @@ def test_volumes_host_path_permissions(self) -> None: rootdir.mkdir() - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - volumes: - /userdir: {userdir.absolute()} - aliases: - doit: - root: true - script: "touch /userdir/test.txt" - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + volumes: + /userdir: {userdir.absolute()} + aliases: + doit: + root: true + script: "touch /userdir/test.txt" + """ + ) try: # Prevent current user from creating directory @@ -1063,14 +1024,13 @@ def test_volumes_host_path_failure(self) -> None: # rootdir is not a dir, it's a file rootdir.write_text("lied about the dir") - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - volumes: - /userdir: {userdir.absolute()} - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + volumes: + /userdir: {userdir.absolute()} + """ + ) run_scuba(["touch", "/userdir/test.txt"], expect_return=128) @@ -1084,14 +1044,13 @@ def test_volumes_host_path_rel(self) -> None: test_message = "Relative paths work" (userdir / "test.txt").write_text(test_message) - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - volumes: - /userdir: ./{userdir} - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + volumes: + /userdir: ./{userdir} + """ + ) # Invoke scuba from a different subdir, for good measure. otherdir = Path("way/down/here") @@ -1126,14 +1085,13 @@ def test_volumes_hostpath_rel_above(self) -> None: # Change to the project subdir and write the .scuba.yml file there. os.chdir(projdir) - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - volumes: - /userdir: ../../../{userdir} - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + volumes: + /userdir: ../../../{userdir} + """ + ) out, _ = run_scuba(["cat", "/userdir/test.txt"]) assert out == test_message @@ -1164,16 +1122,15 @@ def test_volumes_named(self) -> None: TEST_PATH = VOL_PATH / "test.txt" TEST_STR = "it works!" - with open(".scuba.yml", "w") as f: - f.write( - f""" - image: {DOCKER_IMAGE} - hooks: - root: chmod 777 {VOL_PATH} - volumes: - {VOL_PATH}: {self.VOLUME_NAME} - """ - ) + SCUBA_YML.write_text( + f""" + image: {DOCKER_IMAGE} + hooks: + root: chmod 777 {VOL_PATH} + volumes: + {VOL_PATH}: {self.VOLUME_NAME} + """ + ) # Inoke scuba once: Write a file to the named volume run_scuba(["/bin/sh", "-c", f"echo {TEST_STR} > {TEST_PATH}"]) From 7eb95e95ccda0b5b6050a108bfaba57c8ba87464 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 17:30:53 -0400 Subject: [PATCH 07/25] tests: Consistently use Path.write_text() in test_config.py --- tests/test_config.py | 1144 +++++++++++++++++++----------------------- 1 file changed, 522 insertions(+), 622 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 3f1eca8..cc4dfe6 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -13,8 +13,12 @@ from scuba.config import ScubaVolume +SCUBA_YML = Path(".scuba.yml") +GITLAB_YML = Path(".gitlab.yml") + + def load_config() -> scuba.config.ScubaConfig: - return scuba.config.load_config(Path(".scuba.yml"), Path.cwd()) + return scuba.config.load_config(SCUBA_YML, Path.cwd()) class TestCommonScriptSchema: @@ -66,8 +70,7 @@ def _invalid_config(self, match=None): class TestFindConfig(ConfigTest): def test_find_config_cur_dir(self, in_tmp_path) -> None: """find_config can find the config in the current directory""" - with open(".scuba.yml", "w") as f: - f.write("image: bosybux\n") + SCUBA_YML.write_text("image: bosybux") path, rel, _ = scuba.config.find_config() assert_paths_equal(path, in_tmp_path) @@ -75,8 +78,7 @@ def test_find_config_cur_dir(self, in_tmp_path) -> None: def test_find_config_parent_dir(self, in_tmp_path) -> None: """find_config cuba can find the config in the parent directory""" - with open(".scuba.yml", "w") as f: - f.write("image: bosybux\n") + SCUBA_YML.write_text("image: bosybux") os.mkdir("subdir") os.chdir("subdir") @@ -90,8 +92,7 @@ def test_find_config_parent_dir(self, in_tmp_path) -> None: def test_find_config_way_up(self, in_tmp_path) -> None: """find_config can find the config way up the directory hierarchy""" - with open(".scuba.yml", "w") as f: - f.write("image: bosybux\n") + SCUBA_YML.write_text("image: bosybux") subdirs = ["foo", "bar", "snap", "crackle", "pop"] @@ -115,8 +116,7 @@ def test_find_config_nonexist(self) -> None: class TestLoadConfig(ConfigTest): def test_load_config_no_image(self) -> None: """load_config raises ConfigError if the config is empty and image is referenced""" - with open(".scuba.yml", "w") as f: - pass + SCUBA_YML.write_text("") config = load_config() with pytest.raises(scuba.config.ConfigError): @@ -124,27 +124,31 @@ def test_load_config_no_image(self) -> None: def test_load_unexpected_node(self) -> None: """load_config raises ConfigError on unexpected config node""" - with open(".scuba.yml", "w") as f: - f.write("image: bosybux\n") - f.write("unexpected_node_123456: value\n") + SCUBA_YML.write_text( + """ + image: bosybux + unexpected_node_123456: value + """ + ) self._invalid_config() def test_load_config_minimal(self) -> None: """load_config loads a minimal config""" - with open(".scuba.yml", "w") as f: - f.write("image: bosybux\n") - + SCUBA_YML.write_text("image: bosybux") config = load_config() assert config.image == "bosybux" def test_load_config_with_aliases(self) -> None: """load_config loads a config with aliases""" - with open(".scuba.yml", "w") as f: - f.write("image: bosybux\n") - f.write("aliases:\n") - f.write(" foo: bar\n") - f.write(" snap: crackle pop\n") + SCUBA_YML.write_text( + """ + image: bosybux + aliases: + foo: bar + snap: crackle pop + """ + ) config = load_config() assert config.image == "bosybux" @@ -154,33 +158,34 @@ def test_load_config_with_aliases(self) -> None: def test_load_config__no_spaces_in_aliases(self) -> None: """load_config refuses spaces in aliases""" - with open(".scuba.yml", "w") as f: - f.write("image: bosybux\n") - f.write("aliases:\n") - f.write(" this has spaces: whatever\n") + SCUBA_YML.write_text( + """ + image: bosybux + aliases: + this has spaces: whatever + """ + ) self._invalid_config() def test_load_config_image_from_yaml(self) -> None: """load_config loads a config using !from_yaml""" - with open(".gitlab.yml", "w") as f: - f.write("image: dummian:8.2\n") - - with open(".scuba.yml", "w") as f: - f.write("image: !from_yaml .gitlab.yml image\n") + GITLAB_YML.write_text("image: dummian:8.2") + SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML} image") config = load_config() assert config.image == "dummian:8.2" def test_load_config_image_from_yaml_nested_keys(self) -> None: """load_config loads a config using !from_yaml with nested keys""" - with open(".gitlab.yml", "w") as f: - f.write("somewhere:\n") - f.write(" down:\n") - f.write(" here: dummian:8.2\n") - - with open(".scuba.yml", "w") as f: - f.write("image: !from_yaml .gitlab.yml somewhere.down.here\n") + GITLAB_YML.write_text( + """ + somewhere: + down: + here: dummian:8.2 + """ + ) + SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML} somewhere.down.here") config = load_config() assert config.image == "dummian:8.2" @@ -189,80 +194,79 @@ def test_load_config_image_from_yaml_nested_keys_with_escaped_characters( self, ) -> None: """load_config loads a config using !from_yaml with nested keys containing escaped '.' characters""" - with open(".gitlab.yml", "w") as f: - f.write(".its:\n") - f.write(" somewhere.down:\n") - f.write(" here: dummian:8.2\n") - - with open(".scuba.yml", "w") as f: - f.write('image: !from_yaml .gitlab.yml "\\.its.somewhere\\.down.here"\n') + GITLAB_YML.write_text( + """ + .its: + somewhere.down: + here: dummian:8.2 + """ + ) + SCUBA_YML.write_text( + f'image: !from_yaml {GITLAB_YML} "\\.its.somewhere\\.down.here"\n' + ) config = load_config() assert config.image == "dummian:8.2" def test_load_config_from_yaml_cached_file(self) -> None: """load_config loads a config using !from_yaml from cached version""" - with open(".gitlab.yml", "w") as f: - f.write("one: dummian:8.2\n") - f.write("two: dummian:9.3\n") - f.write("three: dummian:10.1\n") - - with open(".scuba.yml", "w") as f: - f.write("image: !from_yaml .gitlab.yml one\n") - f.write("aliases:\n") - f.write(" two:\n") - f.write(" image: !from_yaml .gitlab.yml two\n") - f.write(" script: ugh\n") - f.write(" three:\n") - f.write(" image: !from_yaml .gitlab.yml three\n") - f.write(" script: ugh\n") + GITLAB_YML.write_text( + """ + one: dummian:8.2 + two: dummian:9.3 + three: dummian:10.1 + """ + ) + SCUBA_YML.write_text( + f""" + image: !from_yaml {GITLAB_YML} one + aliases: + two: + image: !from_yaml {GITLAB_YML} two + script: ugh + three: + image: !from_yaml {GITLAB_YML} three + script: ugh + """ + ) with mock.patch.object(Path, "open", autospec=True, side_effect=Path.open) as m: config = load_config() - # Assert that .gitlab.yml was only opened once + # Assert that GITLAB_YML was only opened once assert m.mock_calls == [ - mock.call(Path(".scuba.yml"), "r"), - mock.call(Path(".gitlab.yml"), "r"), + mock.call(SCUBA_YML, "r"), + mock.call(GITLAB_YML, "r"), ] def test_load_config_image_from_yaml_nested_key_missing(self) -> None: """load_config raises ConfigError when !from_yaml references nonexistant key""" - with open(".gitlab.yml", "w") as f: - f.write("somewhere:\n") - f.write(" down:\n") - - with open(".scuba.yml", "w") as f: - f.write("image: !from_yaml .gitlab.yml somewhere.NONEXISTANT\n") - + GITLAB_YML.write_text( + """ + somewhere: + down: + """ + ) + SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML} somewhere.NONEXISTANT") self._invalid_config() def test_load_config_image_from_yaml_missing_file(self) -> None: """load_config raises ConfigError when !from_yaml references nonexistant file""" - with open(".scuba.yml", "w") as f: - f.write("image: !from_yaml .NONEXISTANT.yml image\n") + SCUBA_YML.write_text("image: !from_yaml .NONEXISTANT.yml image") self._invalid_config() def test_load_config_image_from_yaml_unicode_args(self) -> None: """load_config !from_yaml works with unicode args""" - with open(".gitlab.yml", "w") as f: - f.write("𝕦𝕟𝕚𝕔𝕠𝕕𝕖: 𝕨𝕠𝕣𝕜𝕤:𝕠𝕜\n") - - with open(".scuba.yml", "w") as f: - f.write("image: !from_yaml .gitlab.yml 𝕦𝕟𝕚𝕔𝕠𝕕𝕖\n") - + GITLAB_YML.write_text("𝕦𝕟𝕚𝕔𝕠𝕕𝕖: 𝕨𝕠𝕣𝕜𝕤:𝕠𝕜") + SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML} 𝕦𝕟𝕚𝕔𝕠𝕕𝕖") config = load_config() assert config.image == "𝕨𝕠𝕣𝕜𝕤:𝕠𝕜" def test_load_config_image_from_yaml_missing_arg(self) -> None: """load_config raises ConfigError when !from_yaml has missing args""" - with open(".gitlab.yml", "w") as f: - f.write("image: dummian:8.2\n") - - with open(".scuba.yml", "w") as f: - f.write("image: !from_yaml .gitlab.yml\n") - + GITLAB_YML.write_text("image: dummian:8.2") + SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML}") self._invalid_config() def __test_load_config_safe(self, bad_yaml_path) -> None: @@ -281,27 +285,24 @@ def test_load_config_safe(self) -> None: def test_load_config_safe_external(self) -> None: """load_config safely loads yaml from external files""" - with open(".scuba.yml", "w") as f: - f.write("image: !from_yaml .external.yml danger\n") - + SCUBA_YML.write_text("image: !from_yaml .external.yml danger") self.__test_load_config_safe(".external.yml") class TestConfigHooks(ConfigTest): def test_hooks_mixed(self) -> None: """hooks of mixed forms are valid""" - with open(".scuba.yml", "w") as f: - f.write( - """ - image: na - hooks: - root: - script: - - echo "This runs before we switch users" - - id - user: id - """ - ) + SCUBA_YML.write_text( + """ + image: na + hooks: + root: + script: + - echo "This runs before we switch users" + - id + user: id + """ + ) config = load_config() @@ -313,64 +314,58 @@ def test_hooks_mixed(self) -> None: def test_hooks_invalid_list(self) -> None: """hooks with list not under "script" key are invalid""" - with open(".scuba.yml", "w") as f: - f.write( - """ - image: na - hooks: - user: - - this list should be under - - a 'script' - """ - ) - + SCUBA_YML.write_text( + """ + image: na + hooks: + user: + - this list should be under + - a 'script' + """ + ) self._invalid_config() def test_hooks_missing_script(self) -> None: """hooks with dict, but missing "script" are invalid""" - with open(".scuba.yml", "w") as f: - f.write( - """ - image: na - hooks: - user: - not_script: missing "script" key - """ - ) - + SCUBA_YML.write_text( + """ + image: na + hooks: + user: + not_script: missing "script" key + """ + ) self._invalid_config() class TestConfigEnv(ConfigTest): def test_env_invalid(self) -> None: """Environment must be dict or list of strings""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - environment: 666 - """ - ) + SCUBA_YML.write_text( + r""" + image: na + environment: 666 + """ + ) self._invalid_config("must be list or mapping") def test_env_top_dict(self, monkeypatch) -> None: """Top-level environment can be loaded (dict)""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - environment: - FOO: This is foo - FOO_WITH_QUOTES: "\"Quoted foo\"" # Quotes included in value - BAR: "This is bar" - MAGIC: 42 - SWITCH_1: true # YAML boolean - SWITCH_2: "true" # YAML string - EMPTY: "" - EXTERNAL: # Comes from os env - EXTERNAL_NOTSET: # Missing in os env - """ - ) + SCUBA_YML.write_text( + r""" + image: na + environment: + FOO: This is foo + FOO_WITH_QUOTES: "\"Quoted foo\"" # Quotes included in value + BAR: "This is bar" + MAGIC: 42 + SWITCH_1: true # YAML boolean + SWITCH_2: "true" # YAML string + EMPTY: "" + EXTERNAL: # Comes from os env + EXTERNAL_NOTSET: # Missing in os env + """ + ) monkeypatch.setenv("EXTERNAL", "Outside world") monkeypatch.delenv("EXTERNAL_NOTSET", raising=False) @@ -392,21 +387,20 @@ def test_env_top_dict(self, monkeypatch) -> None: def test_env_top_list(self, monkeypatch) -> None: """Top-level environment can be loaded (list)""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - environment: - - FOO=This is foo # No quotes - - FOO_WITH_QUOTES="Quoted foo" # Quotes included in value - - BAR=This is bar - - MAGIC=42 - - SWITCH_2=true - - EMPTY= - - EXTERNAL # Comes from os env - - EXTERNAL_NOTSET # Missing in os env - """ - ) + SCUBA_YML.write_text( + r""" + image: na + environment: + - FOO=This is foo # No quotes + - FOO_WITH_QUOTES="Quoted foo" # Quotes included in value + - BAR=This is bar + - MAGIC=42 + - SWITCH_2=true + - EMPTY= + - EXTERNAL # Comes from os env + - EXTERNAL_NOTSET # Missing in os env + """ + ) monkeypatch.setenv("EXTERNAL", "Outside world") monkeypatch.delenv("EXTERNAL_NOTSET", raising=False) @@ -427,18 +421,17 @@ def test_env_top_list(self, monkeypatch) -> None: def test_env_alias(self) -> None: """Alias can have environment""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - aliases: - al: - script: Don't care - environment: - FOO: Overridden - MORE: Hello world - """ - ) + SCUBA_YML.write_text( + r""" + image: na + aliases: + al: + script: Don't care + environment: + FOO: Overridden + MORE: Hello world + """ + ) config = load_config() @@ -451,118 +444,98 @@ def test_env_alias(self) -> None: class TestConfigEntrypoint(ConfigTest): def test_entrypoint_not_set(self) -> None: """Entrypoint can be missing""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - """ - ) - + SCUBA_YML.write_text("image: na") config = load_config() assert config.entrypoint is None def test_entrypoint_null(self) -> None: """Entrypoint can be set to null""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - entrypoint: - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + entrypoint: + """ + ) config = load_config() assert config.entrypoint == "" # Null => empty string def test_entrypoint_invalid(self) -> None: """Entrypoint of incorrect type raises ConfigError""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - entrypoint: 666 - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + entrypoint: 666 + """ + ) self._invalid_config("must be a string") def test_entrypoint_emptry_string(self) -> None: """Entrypoint can be set to an empty string""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - entrypoint: "" - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + entrypoint: "" + """ + ) config = load_config() assert config.entrypoint == "" def test_entrypoint_set(self) -> None: """Entrypoint can be set""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - entrypoint: my_ep - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + entrypoint: my_ep + """ + ) config = load_config() assert config.entrypoint == "my_ep" def test_alias_entrypoint_null(self) -> None: """Entrypoint can be set to null via alias""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - entrypoint: na_ep - aliases: - testalias: - entrypoint: - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + entrypoint: na_ep + aliases: + testalias: + entrypoint: + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].entrypoint == "" # Null => empty string def test_alias_entrypoint_empty_string(self) -> None: """Entrypoint can be set to an empty string via alias""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - entrypoint: na_ep - aliases: - testalias: - entrypoint: "" - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + entrypoint: na_ep + aliases: + testalias: + entrypoint: "" + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].entrypoint == "" def test_alias_entrypoint(self) -> None: """Entrypoint can be set via alias""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - entrypoint: na_ep - aliases: - testalias: - entrypoint: use_this_ep - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + entrypoint: na_ep + aliases: + testalias: + entrypoint: use_this_ep + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].entrypoint == "use_this_ep" @@ -570,149 +543,125 @@ def test_alias_entrypoint(self) -> None: class TestConfigDockerArgs(ConfigTest): def test_docker_args_not_set(self) -> None: """docker_args can be missing""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - """ - ) - + SCUBA_YML.write_text("image: na") config = load_config() assert config.docker_args is None def test_docker_args_invalid(self) -> None: """docker_args of incorrect type raises ConfigError""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: 666 - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: 666 + """ + ) self._invalid_config("must be a string") def test_docker_args_null(self) -> None: """docker_args can be set to null""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: + """ + ) config = load_config() assert config.docker_args == [] def test_docker_args_set_empty_string(self) -> None: """docker_args can be set to empty string""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: '' - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: '' + """ + ) config = load_config() assert config.docker_args == [] # '' -> [] after shlex.split() def test_docker_args_set(self) -> None: """docker_args can be set""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: --privileged - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: --privileged + """ + ) config = load_config() assert config.docker_args == ["--privileged"] def test_docker_args_set_multi(self) -> None: """docker_args can be set to multiple args""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: --privileged -v /tmp/:/tmp/ - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: --privileged -v /tmp/:/tmp/ + """ + ) config = load_config() assert config.docker_args == ["--privileged", "-v", "/tmp/:/tmp/"] def test_alias_docker_args_null(self) -> None: """docker_args can be set to null via alias""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: --privileged - aliases: - testalias: - docker_args: - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: --privileged + aliases: + testalias: + docker_args: + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].docker_args == [] def test_alias_docker_args_empty_string(self) -> None: """docker_args can be set to empty string via alias""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: --privileged - aliases: - testalias: - docker_args: '' - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: --privileged + aliases: + testalias: + docker_args: '' + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].docker_args == [] def test_alias_docker_args_set(self) -> None: """docker_args can be set via alias""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: --privileged - aliases: - testalias: - docker_args: -v /tmp/:/tmp/ - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: --privileged + aliases: + testalias: + docker_args: -v /tmp/:/tmp/ + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].docker_args == ["-v", "/tmp/:/tmp/"] def test_alias_docker_args_override(self) -> None: """docker_args can be tagged for override""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: --privileged - aliases: - testalias: - docker_args: !override -v /tmp/:/tmp/ - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: --privileged + aliases: + testalias: + docker_args: !override -v /tmp/:/tmp/ + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].docker_args == ["-v", "/tmp/:/tmp/"] assert isinstance( @@ -721,19 +670,17 @@ def test_alias_docker_args_override(self) -> None: def test_alias_docker_args_override_implicit_null(self) -> None: """docker_args can be overridden with an implicit null value""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: --privileged - aliases: - testalias: - docker_args: !override - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: --privileged + aliases: + testalias: + docker_args: !override + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].docker_args == [] assert isinstance( @@ -745,19 +692,17 @@ def test_alias_docker_args_override_from_yaml(self) -> None: with open("args.yml", "w") as f: f.write("args: -v /tmp/:/tmp/\n") - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: --privileged - aliases: - testalias: - docker_args: !override '!from_yaml args.yml args' - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: --privileged + aliases: + testalias: + docker_args: !override '!from_yaml args.yml args' + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].docker_args == ["-v", "/tmp/:/tmp/"] assert isinstance( @@ -769,19 +714,17 @@ def test_alias_docker_args_from_yaml_override(self) -> None: with open("args.yml", "w") as f: f.write("args: !override -v /tmp/:/tmp/\n") - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - docker_args: --privileged - aliases: - testalias: - docker_args: !from_yaml args.yml args - script: - - ugh - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + docker_args: --privileged + aliases: + testalias: + docker_args: !from_yaml args.yml args + script: + - ugh + """ + ) config = load_config() assert config.aliases["testalias"].docker_args == ["-v", "/tmp/:/tmp/"] assert isinstance( @@ -792,97 +735,79 @@ def test_alias_docker_args_from_yaml_override(self) -> None: class TestConfigVolumes(ConfigTest): def test_not_set(self) -> None: """volumes can be missing""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - """ - ) - + SCUBA_YML.write_text("image: na") config = load_config() assert config.volumes is None def test_null(self) -> None: """volumes can be set to null""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + """ + ) config = load_config() assert config.volumes == None def test_invalid_int(self) -> None: """volumes of incorrect type (int) raises ConfigError""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: 666 - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: 666 + """ + ) self._invalid_config("must be a dict") def test_invalid_list(self) -> None: """volume of incorrect type (list) raises ConfigError""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: - - a list makes no sense - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: + - a list makes no sense + """ + ) self._invalid_config("must be string or dict") def test_null_volume_type(self) -> None: """volume of None type raises ConfigError""" # NOTE: In the future, we might want to support this as a volume # (non-bindmount, e.g. '-v /somedata'), or as tmpfs - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /bar: - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + /bar: + """ + ) self._invalid_config("hostpath") def test_complex_missing_hostpath(self) -> None: """volume of incorrect type raises ConfigError""" # NOTE: In the future, we might want to support this as a volume # (non-bindmount, e.g. '-v /somedata'), or as tmpfs - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /bar: - options: hostpath,is,missing - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + /bar: + options: hostpath,is,missing + """ + ) self._invalid_config("hostpath") def test_simple_bind(self) -> None: """volumes can be set using the simple form""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /cpath: /hpath - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + /cpath: /hpath + """ + ) config = load_config() assert config.volumes is not None assert len(config.volumes) == 1 @@ -891,20 +816,18 @@ def test_simple_bind(self) -> None: def test_complex_bind(self) -> None: """volumes can be set using the complex form""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: /host/foo - /bar: - hostpath: /host/bar - /snap: - hostpath: /host/snap - options: z,ro - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: /host/foo + /bar: + hostpath: /host/bar + /snap: + hostpath: /host/snap + options: z,ro + """ + ) config = load_config() vols = config.volumes assert vols is not None @@ -916,16 +839,14 @@ def test_complex_bind(self) -> None: def test_complex_named_volume(self) -> None: """volumes complex form can specify a named volume""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: - name: foo-volume - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: + name: foo-volume + """ + ) config = load_config() assert config.volumes is not None assert len(config.volumes) == 1 @@ -937,22 +858,20 @@ def test_complex_named_volume(self) -> None: def test_via_alias(self) -> None: """volumes can be set via alias""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - aliases: - testalias: - script: - - ugh - volumes: - /foo: /host/foo - /bar: - hostpath: /host/bar - options: z,ro - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + aliases: + testalias: + script: + - ugh + volumes: + /foo: /host/foo + /bar: + hostpath: /host/bar + options: z,ro + """ + ) config = load_config() vols = config.aliases["testalias"].volumes assert vols is not None @@ -965,15 +884,13 @@ def test_with_env_vars_simple(self, monkeypatch) -> None: """volume definitions can contain environment variables""" monkeypatch.setenv("TEST_VOL_PATH", "/bar/baz") monkeypatch.setenv("TEST_VOL_PATH2", "/moo/doo") - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - $TEST_VOL_PATH/foo: ${TEST_VOL_PATH2}/foo - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + $TEST_VOL_PATH/foo: ${TEST_VOL_PATH2}/foo + """ + ) config = load_config() vols = config.volumes assert vols is not None @@ -987,20 +904,18 @@ def test_with_env_vars_complex(self, monkeypatch) -> None: monkeypatch.setenv("TEST_TMP", "/tmp") monkeypatch.setenv("TEST_MAIL", "/var/spool/mail/testuser") - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - $TEST_HOME/.config: ${TEST_HOME}/.config - $TEST_TMP/: - hostpath: $TEST_HOME/scuba/myproject/tmp - /var/spool/mail/container: - hostpath: $TEST_MAIL - options: z,ro - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + $TEST_HOME/.config: ${TEST_HOME}/.config + $TEST_TMP/: + hostpath: $TEST_HOME/scuba/myproject/tmp + /var/spool/mail/container: + hostpath: $TEST_MAIL + options: z,ro + """ + ) config = load_config() vols = config.volumes assert vols is not None @@ -1016,31 +931,29 @@ def test_with_invalid_env_vars(self, monkeypatch) -> None: """Volume definitions cannot include unset env vars""" # Ensure that the entry does not exist in the environment monkeypatch.delenv("TEST_VAR1", raising=False) - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - $TEST_VAR1/foo: /host/foo - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + $TEST_VAR1/foo: /host/foo + """ + ) self._invalid_config("TEST_VAR1") def test_hostpath_rel(self, monkeypatch, in_tmp_path) -> None: """volume hostpath can be relative to scuba_root (top dir)""" monkeypatch.setenv("RELVAR", "./spam/eggs") - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /bar: ./foo/bar # simple form, dotted path - /scp: # complex form - hostpath: ./snap/crackle/pop - /relvar: $RELVAR # simple form, relative path in variable - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + /bar: ./foo/bar # simple form, dotted path + /scp: # complex form + hostpath: ./snap/crackle/pop + /relvar: $RELVAR # simple form, relative path in variable + """ + ) # Make a subdirectory and cd into it subdir = Path("way/down/here") @@ -1074,14 +987,13 @@ def test_hostpath_rel_above(self, monkeypatch, in_tmp_path) -> None: monkeypatch.chdir(project_dir) # Now put .scuba.yml here - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: ../../../foo_up_here - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: ../../../foo_up_here + """ + ) # Locate the config found_topdir, found_rel, config = scuba.config.find_config() @@ -1093,29 +1005,26 @@ def test_hostpath_rel_above(self, monkeypatch, in_tmp_path) -> None: def test_hostpath_rel_requires_dot_complex(self, monkeypatch, in_tmp_path) -> None: """relaitve volume hostpath (complex form) must start with ./ or ../""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /one: - hostpath: foo # Forbidden - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + /one: + hostpath: foo # Forbidden + """ + ) self._invalid_config("Relative path must start with ./ or ../") def test_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: """volume definitions can contain environment variables, including relative path portions""" monkeypatch.setenv("PREFIX", "./") - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: ${PREFIX}/foo - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: ${PREFIX}/foo + """ + ) config = load_config() vols = config.volumes assert vols is not None @@ -1124,27 +1033,24 @@ def test_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: assert_vol(vols, "/foo", in_tmp_path / "foo") def test_contpath_rel(self, monkeypatch, in_tmp_path) -> None: - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - foo: /what/now - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + foo: /what/now + """ + ) self._invalid_config("Relative path not allowed: foo") def test_simple_named_volume(self) -> None: """volumes simple form can specify a named volume""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: foo-volume - """ - ) - + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: foo-volume + """ + ) config = load_config() assert config.volumes is not None assert len(config.volumes) == 1 @@ -1156,14 +1062,13 @@ def test_simple_named_volume(self) -> None: def test_simple_named_volume_env(self, monkeypatch) -> None: """volumes simple form can specify a named volume via env var""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: $FOO_VOLUME - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: $FOO_VOLUME + """ + ) monkeypatch.setenv("FOO_VOLUME", "foo-volume") @@ -1178,15 +1083,14 @@ def test_simple_named_volume_env(self, monkeypatch) -> None: def test_complex_named_volume_env(self, monkeypatch) -> None: """volumes complex form can specify a named volume via env var""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: - name: $FOO_VOLUME - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: + name: $FOO_VOLUME + """ + ) monkeypatch.setenv("FOO_VOLUME", "foo-volume") @@ -1201,56 +1105,52 @@ def test_complex_named_volume_env(self, monkeypatch) -> None: def test_complex_named_volume_env_unset(self) -> None: """volumes complex form fails on unset env var""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: - name: $FOO_VOLUME - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: + name: $FOO_VOLUME + """ + ) self._invalid_config("Unset environment variable") def test_complex_invalid_hostpath(self) -> None: """volumes complex form cannot specify an invalid hostpath""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: - hostpath: foo-volume - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: + hostpath: foo-volume + """ + ) self._invalid_config("Relative path must start with ./ or ../") def test_complex_hostpath_and_name(self) -> None: """volumes complex form cannot specify a named volume and hostpath""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: - hostpath: /bar - name: foo-volume - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: + hostpath: /bar + name: foo-volume + """ + ) self._invalid_config( "Volume /foo must have exactly one of 'hostpath' or 'name' subkey" ) def test_complex_empty(self) -> None: """volumes complex form cannot be empty""" - with open(".scuba.yml", "w") as f: - f.write( - r""" - image: na - volumes: - /foo: - """ - ) + SCUBA_YML.write_text( + r""" + image: na + volumes: + /foo: + """ + ) self._invalid_config( "Volume /foo must have exactly one of 'hostpath' or 'name' subkey" ) From 8e2d5986ea976e8c234821d6560309e944f2be06 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 18:38:52 -0400 Subject: [PATCH 08/25] tests: Move ConfigTest._invalid_config() to free function Also, make error_match a keyword-only argument. --- tests/test_config.py | 55 +++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index cc4dfe6..0dc29bb 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -21,6 +21,11 @@ def load_config() -> scuba.config.ScubaConfig: return scuba.config.load_config(SCUBA_YML, Path.cwd()) +def invalid_config(*, error_match=None): + with pytest.raises(scuba.config.ConfigError, match=error_match) as e: + load_config() + + class TestCommonScriptSchema: def test_simple(self) -> None: """Simple form: value is a string""" @@ -62,9 +67,7 @@ def test_script_key_mapping_invalid(self) -> None: @pytest.mark.usefixtures("in_tmp_path") class ConfigTest: - def _invalid_config(self, match=None): - with pytest.raises(scuba.config.ConfigError, match=match) as e: - load_config() + pass class TestFindConfig(ConfigTest): @@ -131,7 +134,7 @@ def test_load_unexpected_node(self) -> None: """ ) - self._invalid_config() + invalid_config() def test_load_config_minimal(self) -> None: """load_config loads a minimal config""" @@ -166,7 +169,7 @@ def test_load_config__no_spaces_in_aliases(self) -> None: """ ) - self._invalid_config() + invalid_config() def test_load_config_image_from_yaml(self) -> None: """load_config loads a config using !from_yaml""" @@ -248,13 +251,13 @@ def test_load_config_image_from_yaml_nested_key_missing(self) -> None: """ ) SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML} somewhere.NONEXISTANT") - self._invalid_config() + invalid_config() def test_load_config_image_from_yaml_missing_file(self) -> None: """load_config raises ConfigError when !from_yaml references nonexistant file""" SCUBA_YML.write_text("image: !from_yaml .NONEXISTANT.yml image") - self._invalid_config() + invalid_config() def test_load_config_image_from_yaml_unicode_args(self) -> None: """load_config !from_yaml works with unicode args""" @@ -267,7 +270,7 @@ def test_load_config_image_from_yaml_missing_arg(self) -> None: """load_config raises ConfigError when !from_yaml has missing args""" GITLAB_YML.write_text("image: dummian:8.2") SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML}") - self._invalid_config() + invalid_config() def __test_load_config_safe(self, bad_yaml_path) -> None: with open(bad_yaml_path, "w") as f: @@ -323,7 +326,7 @@ def test_hooks_invalid_list(self) -> None: - a 'script' """ ) - self._invalid_config() + invalid_config() def test_hooks_missing_script(self) -> None: """hooks with dict, but missing "script" are invalid""" @@ -335,7 +338,7 @@ def test_hooks_missing_script(self) -> None: not_script: missing "script" key """ ) - self._invalid_config() + invalid_config() class TestConfigEnv(ConfigTest): @@ -347,7 +350,7 @@ def test_env_invalid(self) -> None: environment: 666 """ ) - self._invalid_config("must be list or mapping") + invalid_config(error_match="must be list or mapping") def test_env_top_dict(self, monkeypatch) -> None: """Top-level environment can be loaded (dict)""" @@ -467,7 +470,7 @@ def test_entrypoint_invalid(self) -> None: entrypoint: 666 """ ) - self._invalid_config("must be a string") + invalid_config(error_match="must be a string") def test_entrypoint_emptry_string(self) -> None: """Entrypoint can be set to an empty string""" @@ -555,7 +558,7 @@ def test_docker_args_invalid(self) -> None: docker_args: 666 """ ) - self._invalid_config("must be a string") + invalid_config(error_match="must be a string") def test_docker_args_null(self) -> None: """docker_args can be set to null""" @@ -758,7 +761,7 @@ def test_invalid_int(self) -> None: volumes: 666 """ ) - self._invalid_config("must be a dict") + invalid_config(error_match="must be a dict") def test_invalid_list(self) -> None: """volume of incorrect type (list) raises ConfigError""" @@ -770,7 +773,7 @@ def test_invalid_list(self) -> None: - a list makes no sense """ ) - self._invalid_config("must be string or dict") + invalid_config(error_match="must be string or dict") def test_null_volume_type(self) -> None: """volume of None type raises ConfigError""" @@ -783,7 +786,7 @@ def test_null_volume_type(self) -> None: /bar: """ ) - self._invalid_config("hostpath") + invalid_config(error_match="hostpath") def test_complex_missing_hostpath(self) -> None: """volume of incorrect type raises ConfigError""" @@ -797,7 +800,7 @@ def test_complex_missing_hostpath(self) -> None: options: hostpath,is,missing """ ) - self._invalid_config("hostpath") + invalid_config(error_match="hostpath") def test_simple_bind(self) -> None: """volumes can be set using the simple form""" @@ -938,7 +941,7 @@ def test_with_invalid_env_vars(self, monkeypatch) -> None: $TEST_VAR1/foo: /host/foo """ ) - self._invalid_config("TEST_VAR1") + invalid_config(error_match="TEST_VAR1") def test_hostpath_rel(self, monkeypatch, in_tmp_path) -> None: """volume hostpath can be relative to scuba_root (top dir)""" @@ -1013,7 +1016,7 @@ def test_hostpath_rel_requires_dot_complex(self, monkeypatch, in_tmp_path) -> No hostpath: foo # Forbidden """ ) - self._invalid_config("Relative path must start with ./ or ../") + invalid_config(error_match="Relative path must start with ./ or ../") def test_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: """volume definitions can contain environment variables, including relative path portions""" @@ -1040,7 +1043,7 @@ def test_contpath_rel(self, monkeypatch, in_tmp_path) -> None: foo: /what/now """ ) - self._invalid_config("Relative path not allowed: foo") + invalid_config(error_match="Relative path not allowed: foo") def test_simple_named_volume(self) -> None: """volumes simple form can specify a named volume""" @@ -1113,7 +1116,7 @@ def test_complex_named_volume_env_unset(self) -> None: name: $FOO_VOLUME """ ) - self._invalid_config("Unset environment variable") + invalid_config(error_match="Unset environment variable") def test_complex_invalid_hostpath(self) -> None: """volumes complex form cannot specify an invalid hostpath""" @@ -1125,7 +1128,7 @@ def test_complex_invalid_hostpath(self) -> None: hostpath: foo-volume """ ) - self._invalid_config("Relative path must start with ./ or ../") + invalid_config(error_match="Relative path must start with ./ or ../") def test_complex_hostpath_and_name(self) -> None: """volumes complex form cannot specify a named volume and hostpath""" @@ -1138,8 +1141,8 @@ def test_complex_hostpath_and_name(self) -> None: name: foo-volume """ ) - self._invalid_config( - "Volume /foo must have exactly one of 'hostpath' or 'name' subkey" + invalid_config( + error_match="Volume /foo must have exactly one of 'hostpath' or 'name' subkey" ) def test_complex_empty(self) -> None: @@ -1151,6 +1154,6 @@ def test_complex_empty(self) -> None: /foo: """ ) - self._invalid_config( - "Volume /foo must have exactly one of 'hostpath' or 'name' subkey" + invalid_config( + error_match="Volume /foo must have exactly one of 'hostpath' or 'name' subkey" ) From 637d0856138a1a19131e62f2cfe6c395f7d788a3 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 19:31:21 -0400 Subject: [PATCH 09/25] tests: Pass config text to load_config() and invalid_config() --- tests/test_config.py | 393 ++++++++++++++++++------------------------- 1 file changed, 165 insertions(+), 228 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 0dc29bb..224b2e8 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -5,6 +5,7 @@ from pathlib import Path import pytest from shutil import rmtree +from typing import Optional from unittest import mock from .utils import assert_paths_equal, assert_vol @@ -17,13 +18,19 @@ GITLAB_YML = Path(".gitlab.yml") -def load_config() -> scuba.config.ScubaConfig: +def load_config(*, config_text: Optional[str] = None) -> scuba.config.ScubaConfig: + if config_text is not None: + SCUBA_YML.write_text(config_text) return scuba.config.load_config(SCUBA_YML, Path.cwd()) -def invalid_config(*, error_match=None): +def invalid_config( + *, + config_text: Optional[str] = None, + error_match: Optional[str] = None, +) -> None: with pytest.raises(scuba.config.ConfigError, match=error_match) as e: - load_config() + load_config(config_text=config_text) class TestCommonScriptSchema: @@ -119,41 +126,34 @@ def test_find_config_nonexist(self) -> None: class TestLoadConfig(ConfigTest): def test_load_config_no_image(self) -> None: """load_config raises ConfigError if the config is empty and image is referenced""" - SCUBA_YML.write_text("") - - config = load_config() + config = load_config(config_text="") with pytest.raises(scuba.config.ConfigError): img = config.image def test_load_unexpected_node(self) -> None: """load_config raises ConfigError on unexpected config node""" - SCUBA_YML.write_text( - """ + invalid_config( + config_text=""" image: bosybux unexpected_node_123456: value """ ) - invalid_config() - def test_load_config_minimal(self) -> None: """load_config loads a minimal config""" - SCUBA_YML.write_text("image: bosybux") - config = load_config() + config = load_config(config_text="image: bosybux") assert config.image == "bosybux" def test_load_config_with_aliases(self) -> None: """load_config loads a config with aliases""" - SCUBA_YML.write_text( - """ + config = load_config( + config_text=""" image: bosybux aliases: foo: bar snap: crackle pop """ ) - - config = load_config() assert config.image == "bosybux" assert len(config.aliases) == 2 assert config.aliases["foo"].script == ["bar"] @@ -161,22 +161,18 @@ def test_load_config_with_aliases(self) -> None: def test_load_config__no_spaces_in_aliases(self) -> None: """load_config refuses spaces in aliases""" - SCUBA_YML.write_text( - """ + invalid_config( + config_text=""" image: bosybux aliases: this has spaces: whatever """ ) - invalid_config() - def test_load_config_image_from_yaml(self) -> None: """load_config loads a config using !from_yaml""" GITLAB_YML.write_text("image: dummian:8.2") - SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML} image") - - config = load_config() + config = load_config(config_text=f"image: !from_yaml {GITLAB_YML} image") assert config.image == "dummian:8.2" def test_load_config_image_from_yaml_nested_keys(self) -> None: @@ -188,9 +184,9 @@ def test_load_config_image_from_yaml_nested_keys(self) -> None: here: dummian:8.2 """ ) - SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML} somewhere.down.here") - - config = load_config() + config = load_config( + config_text=f"image: !from_yaml {GITLAB_YML} somewhere.down.here" + ) assert config.image == "dummian:8.2" def test_load_config_image_from_yaml_nested_keys_with_escaped_characters( @@ -204,11 +200,9 @@ def test_load_config_image_from_yaml_nested_keys_with_escaped_characters( here: dummian:8.2 """ ) - SCUBA_YML.write_text( - f'image: !from_yaml {GITLAB_YML} "\\.its.somewhere\\.down.here"\n' + config = load_config( + config_text=f'image: !from_yaml {GITLAB_YML} "\\.its.somewhere\\.down.here"\n' ) - - config = load_config() assert config.image == "dummian:8.2" def test_load_config_from_yaml_cached_file(self) -> None: @@ -234,6 +228,7 @@ def test_load_config_from_yaml_cached_file(self) -> None: ) with mock.patch.object(Path, "open", autospec=True, side_effect=Path.open) as m: + # Write config separately so its open() call is not counted here. config = load_config() # Assert that GITLAB_YML was only opened once @@ -250,27 +245,24 @@ def test_load_config_image_from_yaml_nested_key_missing(self) -> None: down: """ ) - SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML} somewhere.NONEXISTANT") - invalid_config() + invalid_config( + config_text=f"image: !from_yaml {GITLAB_YML} somewhere.NONEXISTANT" + ) def test_load_config_image_from_yaml_missing_file(self) -> None: """load_config raises ConfigError when !from_yaml references nonexistant file""" - SCUBA_YML.write_text("image: !from_yaml .NONEXISTANT.yml image") - - invalid_config() + invalid_config(config_text="image: !from_yaml .NONEXISTANT.yml image") def test_load_config_image_from_yaml_unicode_args(self) -> None: """load_config !from_yaml works with unicode args""" GITLAB_YML.write_text("𝕦𝕟𝕚𝕔𝕠𝕕𝕖: 𝕨𝕠𝕣𝕜𝕤:𝕠𝕜") - SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML} 𝕦𝕟𝕚𝕔𝕠𝕕𝕖") - config = load_config() + config = load_config(config_text=f"image: !from_yaml {GITLAB_YML} 𝕦𝕟𝕚𝕔𝕠𝕕𝕖") assert config.image == "𝕨𝕠𝕣𝕜𝕤:𝕠𝕜" def test_load_config_image_from_yaml_missing_arg(self) -> None: """load_config raises ConfigError when !from_yaml has missing args""" GITLAB_YML.write_text("image: dummian:8.2") - SCUBA_YML.write_text(f"image: !from_yaml {GITLAB_YML}") - invalid_config() + invalid_config(config_text=f"image: !from_yaml {GITLAB_YML}") def __test_load_config_safe(self, bad_yaml_path) -> None: with open(bad_yaml_path, "w") as f: @@ -295,8 +287,8 @@ def test_load_config_safe_external(self) -> None: class TestConfigHooks(ConfigTest): def test_hooks_mixed(self) -> None: """hooks of mixed forms are valid""" - SCUBA_YML.write_text( - """ + config = load_config( + config_text=""" image: na hooks: root: @@ -306,9 +298,6 @@ def test_hooks_mixed(self) -> None: user: id """ ) - - config = load_config() - assert config.hooks.get("root") == [ 'echo "This runs before we switch users"', "id", @@ -317,8 +306,8 @@ def test_hooks_mixed(self) -> None: def test_hooks_invalid_list(self) -> None: """hooks with list not under "script" key are invalid""" - SCUBA_YML.write_text( - """ + invalid_config( + config_text=""" image: na hooks: user: @@ -326,36 +315,36 @@ def test_hooks_invalid_list(self) -> None: - a 'script' """ ) - invalid_config() def test_hooks_missing_script(self) -> None: """hooks with dict, but missing "script" are invalid""" - SCUBA_YML.write_text( - """ + invalid_config( + config_text=""" image: na hooks: user: not_script: missing "script" key """ ) - invalid_config() class TestConfigEnv(ConfigTest): def test_env_invalid(self) -> None: """Environment must be dict or list of strings""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na environment: 666 - """ + """, + error_match="must be list or mapping", ) - invalid_config(error_match="must be list or mapping") def test_env_top_dict(self, monkeypatch) -> None: """Top-level environment can be loaded (dict)""" - SCUBA_YML.write_text( - r""" + monkeypatch.setenv("EXTERNAL", "Outside world") + monkeypatch.delenv("EXTERNAL_NOTSET", raising=False) + config = load_config( + config_text=r""" image: na environment: FOO: This is foo @@ -369,12 +358,6 @@ def test_env_top_dict(self, monkeypatch) -> None: EXTERNAL_NOTSET: # Missing in os env """ ) - - monkeypatch.setenv("EXTERNAL", "Outside world") - monkeypatch.delenv("EXTERNAL_NOTSET", raising=False) - - config = load_config() - expect = dict( FOO="This is foo", FOO_WITH_QUOTES='"Quoted foo"', @@ -390,8 +373,10 @@ def test_env_top_dict(self, monkeypatch) -> None: def test_env_top_list(self, monkeypatch) -> None: """Top-level environment can be loaded (list)""" - SCUBA_YML.write_text( - r""" + monkeypatch.setenv("EXTERNAL", "Outside world") + monkeypatch.delenv("EXTERNAL_NOTSET", raising=False) + config = load_config( + config_text=r""" image: na environment: - FOO=This is foo # No quotes @@ -404,12 +389,6 @@ def test_env_top_list(self, monkeypatch) -> None: - EXTERNAL_NOTSET # Missing in os env """ ) - - monkeypatch.setenv("EXTERNAL", "Outside world") - monkeypatch.delenv("EXTERNAL_NOTSET", raising=False) - - config = load_config() - expect = dict( FOO="This is foo", FOO_WITH_QUOTES='"Quoted foo"', @@ -424,8 +403,8 @@ def test_env_top_list(self, monkeypatch) -> None: def test_env_alias(self) -> None: """Alias can have environment""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na aliases: al: @@ -435,9 +414,6 @@ def test_env_alias(self) -> None: MORE: Hello world """ ) - - config = load_config() - assert config.aliases["al"].environment == dict( FOO="Overridden", MORE="Hello world", @@ -447,57 +423,53 @@ def test_env_alias(self) -> None: class TestConfigEntrypoint(ConfigTest): def test_entrypoint_not_set(self) -> None: """Entrypoint can be missing""" - SCUBA_YML.write_text("image: na") - config = load_config() + config = load_config(config_text="image: na") assert config.entrypoint is None def test_entrypoint_null(self) -> None: """Entrypoint can be set to null""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na entrypoint: """ ) - config = load_config() assert config.entrypoint == "" # Null => empty string def test_entrypoint_invalid(self) -> None: """Entrypoint of incorrect type raises ConfigError""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na entrypoint: 666 - """ + """, + error_match="must be a string", ) - invalid_config(error_match="must be a string") - def test_entrypoint_emptry_string(self) -> None: + def test_entrypoint_empty_string(self) -> None: """Entrypoint can be set to an empty string""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na entrypoint: "" """ ) - config = load_config() assert config.entrypoint == "" def test_entrypoint_set(self) -> None: """Entrypoint can be set""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na entrypoint: my_ep """ ) - config = load_config() assert config.entrypoint == "my_ep" def test_alias_entrypoint_null(self) -> None: """Entrypoint can be set to null via alias""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na entrypoint: na_ep aliases: @@ -507,13 +479,12 @@ def test_alias_entrypoint_null(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].entrypoint == "" # Null => empty string def test_alias_entrypoint_empty_string(self) -> None: """Entrypoint can be set to an empty string via alias""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na entrypoint: na_ep aliases: @@ -523,13 +494,12 @@ def test_alias_entrypoint_empty_string(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].entrypoint == "" def test_alias_entrypoint(self) -> None: """Entrypoint can be set via alias""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na entrypoint: na_ep aliases: @@ -539,75 +509,69 @@ def test_alias_entrypoint(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].entrypoint == "use_this_ep" class TestConfigDockerArgs(ConfigTest): def test_docker_args_not_set(self) -> None: """docker_args can be missing""" - SCUBA_YML.write_text("image: na") - config = load_config() + config = load_config(config_text="image: na") assert config.docker_args is None def test_docker_args_invalid(self) -> None: """docker_args of incorrect type raises ConfigError""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na docker_args: 666 - """ + """, + error_match="must be a string", ) - invalid_config(error_match="must be a string") def test_docker_args_null(self) -> None: """docker_args can be set to null""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: """ ) - config = load_config() assert config.docker_args == [] def test_docker_args_set_empty_string(self) -> None: """docker_args can be set to empty string""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: '' """ ) - config = load_config() assert config.docker_args == [] # '' -> [] after shlex.split() def test_docker_args_set(self) -> None: """docker_args can be set""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: --privileged """ ) - config = load_config() assert config.docker_args == ["--privileged"] def test_docker_args_set_multi(self) -> None: """docker_args can be set to multiple args""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: --privileged -v /tmp/:/tmp/ """ ) - config = load_config() assert config.docker_args == ["--privileged", "-v", "/tmp/:/tmp/"] def test_alias_docker_args_null(self) -> None: """docker_args can be set to null via alias""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: --privileged aliases: @@ -617,13 +581,12 @@ def test_alias_docker_args_null(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].docker_args == [] def test_alias_docker_args_empty_string(self) -> None: """docker_args can be set to empty string via alias""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: --privileged aliases: @@ -633,13 +596,12 @@ def test_alias_docker_args_empty_string(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].docker_args == [] def test_alias_docker_args_set(self) -> None: """docker_args can be set via alias""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: --privileged aliases: @@ -649,13 +611,12 @@ def test_alias_docker_args_set(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].docker_args == ["-v", "/tmp/:/tmp/"] def test_alias_docker_args_override(self) -> None: """docker_args can be tagged for override""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: --privileged aliases: @@ -665,7 +626,6 @@ def test_alias_docker_args_override(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].docker_args == ["-v", "/tmp/:/tmp/"] assert isinstance( config.aliases["testalias"].docker_args, scuba.config.OverrideMixin @@ -673,8 +633,8 @@ def test_alias_docker_args_override(self) -> None: def test_alias_docker_args_override_implicit_null(self) -> None: """docker_args can be overridden with an implicit null value""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: --privileged aliases: @@ -684,7 +644,6 @@ def test_alias_docker_args_override_implicit_null(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].docker_args == [] assert isinstance( config.aliases["testalias"].docker_args, scuba.config.OverrideMixin @@ -695,8 +654,8 @@ def test_alias_docker_args_override_from_yaml(self) -> None: with open("args.yml", "w") as f: f.write("args: -v /tmp/:/tmp/\n") - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: --privileged aliases: @@ -706,7 +665,6 @@ def test_alias_docker_args_override_from_yaml(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].docker_args == ["-v", "/tmp/:/tmp/"] assert isinstance( config.aliases["testalias"].docker_args, scuba.config.OverrideMixin @@ -717,8 +675,8 @@ def test_alias_docker_args_from_yaml_override(self) -> None: with open("args.yml", "w") as f: f.write("args: !override -v /tmp/:/tmp/\n") - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na docker_args: --privileged aliases: @@ -728,7 +686,6 @@ def test_alias_docker_args_from_yaml_override(self) -> None: - ugh """ ) - config = load_config() assert config.aliases["testalias"].docker_args == ["-v", "/tmp/:/tmp/"] assert isinstance( config.aliases["testalias"].docker_args, scuba.config.OverrideMixin @@ -738,80 +695,77 @@ def test_alias_docker_args_from_yaml_override(self) -> None: class TestConfigVolumes(ConfigTest): def test_not_set(self) -> None: """volumes can be missing""" - SCUBA_YML.write_text("image: na") - config = load_config() + config = load_config(config_text="image: na") assert config.volumes is None def test_null(self) -> None: """volumes can be set to null""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na volumes: """ ) - config = load_config() assert config.volumes == None def test_invalid_int(self) -> None: """volumes of incorrect type (int) raises ConfigError""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: 666 - """ + """, + error_match="must be a dict", ) - invalid_config(error_match="must be a dict") def test_invalid_list(self) -> None: """volume of incorrect type (list) raises ConfigError""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: /foo: - a list makes no sense - """ + """, + error_match="must be string or dict", ) - invalid_config(error_match="must be string or dict") def test_null_volume_type(self) -> None: """volume of None type raises ConfigError""" # NOTE: In the future, we might want to support this as a volume # (non-bindmount, e.g. '-v /somedata'), or as tmpfs - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: /bar: - """ + """, + error_match="hostpath", ) - invalid_config(error_match="hostpath") def test_complex_missing_hostpath(self) -> None: """volume of incorrect type raises ConfigError""" # NOTE: In the future, we might want to support this as a volume # (non-bindmount, e.g. '-v /somedata'), or as tmpfs - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: /bar: options: hostpath,is,missing - """ + """, + error_match="hostpath", ) - invalid_config(error_match="hostpath") def test_simple_bind(self) -> None: """volumes can be set using the simple form""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na volumes: /cpath: /hpath """ ) - config = load_config() assert config.volumes is not None assert len(config.volumes) == 1 @@ -819,8 +773,8 @@ def test_simple_bind(self) -> None: def test_complex_bind(self) -> None: """volumes can be set using the complex form""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na volumes: /foo: /host/foo @@ -831,7 +785,6 @@ def test_complex_bind(self) -> None: options: z,ro """ ) - config = load_config() vols = config.volumes assert vols is not None assert len(vols) == 3 @@ -842,15 +795,14 @@ def test_complex_bind(self) -> None: def test_complex_named_volume(self) -> None: """volumes complex form can specify a named volume""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na volumes: /foo: name: foo-volume """ ) - config = load_config() assert config.volumes is not None assert len(config.volumes) == 1 vol = config.volumes[Path("/foo")] @@ -861,8 +813,8 @@ def test_complex_named_volume(self) -> None: def test_via_alias(self) -> None: """volumes can be set via alias""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na aliases: testalias: @@ -875,7 +827,6 @@ def test_via_alias(self) -> None: options: z,ro """ ) - config = load_config() vols = config.aliases["testalias"].volumes assert vols is not None assert len(vols) == 2 @@ -887,14 +838,13 @@ def test_with_env_vars_simple(self, monkeypatch) -> None: """volume definitions can contain environment variables""" monkeypatch.setenv("TEST_VOL_PATH", "/bar/baz") monkeypatch.setenv("TEST_VOL_PATH2", "/moo/doo") - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na volumes: $TEST_VOL_PATH/foo: ${TEST_VOL_PATH2}/foo """ ) - config = load_config() vols = config.volumes assert vols is not None assert len(vols) == 1 @@ -907,8 +857,8 @@ def test_with_env_vars_complex(self, monkeypatch) -> None: monkeypatch.setenv("TEST_TMP", "/tmp") monkeypatch.setenv("TEST_MAIL", "/var/spool/mail/testuser") - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na volumes: $TEST_HOME/.config: ${TEST_HOME}/.config @@ -919,7 +869,6 @@ def test_with_env_vars_complex(self, monkeypatch) -> None: options: z,ro """ ) - config = load_config() vols = config.volumes assert vols is not None assert len(vols) == 3 @@ -934,14 +883,14 @@ def test_with_invalid_env_vars(self, monkeypatch) -> None: """Volume definitions cannot include unset env vars""" # Ensure that the entry does not exist in the environment monkeypatch.delenv("TEST_VAR1", raising=False) - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: $TEST_VAR1/foo: /host/foo - """ + """, + error_match="TEST_VAR1", ) - invalid_config(error_match="TEST_VAR1") def test_hostpath_rel(self, monkeypatch, in_tmp_path) -> None: """volume hostpath can be relative to scuba_root (top dir)""" @@ -1008,27 +957,26 @@ def test_hostpath_rel_above(self, monkeypatch, in_tmp_path) -> None: def test_hostpath_rel_requires_dot_complex(self, monkeypatch, in_tmp_path) -> None: """relaitve volume hostpath (complex form) must start with ./ or ../""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: /one: hostpath: foo # Forbidden - """ + """, + error_match="Relative path must start with ./ or ../", ) - invalid_config(error_match="Relative path must start with ./ or ../") def test_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: """volume definitions can contain environment variables, including relative path portions""" monkeypatch.setenv("PREFIX", "./") - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na volumes: /foo: ${PREFIX}/foo """ ) - config = load_config() vols = config.volumes assert vols is not None assert len(vols) == 1 @@ -1036,25 +984,24 @@ def test_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: assert_vol(vols, "/foo", in_tmp_path / "foo") def test_contpath_rel(self, monkeypatch, in_tmp_path) -> None: - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: foo: /what/now - """ + """, + error_match="Relative path not allowed: foo", ) - invalid_config(error_match="Relative path not allowed: foo") def test_simple_named_volume(self) -> None: """volumes simple form can specify a named volume""" - SCUBA_YML.write_text( - r""" + config = load_config( + config_text=r""" image: na volumes: /foo: foo-volume """ ) - config = load_config() assert config.volumes is not None assert len(config.volumes) == 1 vol = config.volumes[Path("/foo")] @@ -1065,17 +1012,14 @@ def test_simple_named_volume(self) -> None: def test_simple_named_volume_env(self, monkeypatch) -> None: """volumes simple form can specify a named volume via env var""" - SCUBA_YML.write_text( - r""" + monkeypatch.setenv("FOO_VOLUME", "foo-volume") + config = load_config( + config_text=r""" image: na volumes: /foo: $FOO_VOLUME """ ) - - monkeypatch.setenv("FOO_VOLUME", "foo-volume") - - config = load_config() assert config.volumes is not None assert len(config.volumes) == 1 vol = config.volumes[Path("/foo")] @@ -1086,18 +1030,15 @@ def test_simple_named_volume_env(self, monkeypatch) -> None: def test_complex_named_volume_env(self, monkeypatch) -> None: """volumes complex form can specify a named volume via env var""" - SCUBA_YML.write_text( - r""" + monkeypatch.setenv("FOO_VOLUME", "foo-volume") + config = load_config( + config_text=r""" image: na volumes: /foo: name: $FOO_VOLUME """ ) - - monkeypatch.setenv("FOO_VOLUME", "foo-volume") - - config = load_config() assert config.volumes is not None assert len(config.volumes) == 1 vol = config.volumes[Path("/foo")] @@ -1108,52 +1049,48 @@ def test_complex_named_volume_env(self, monkeypatch) -> None: def test_complex_named_volume_env_unset(self) -> None: """volumes complex form fails on unset env var""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: /foo: name: $FOO_VOLUME - """ + """, + error_match="Unset environment variable", ) - invalid_config(error_match="Unset environment variable") def test_complex_invalid_hostpath(self) -> None: """volumes complex form cannot specify an invalid hostpath""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: /foo: hostpath: foo-volume - """ + """, + error_match="Relative path must start with ./ or ../", ) - invalid_config(error_match="Relative path must start with ./ or ../") def test_complex_hostpath_and_name(self) -> None: """volumes complex form cannot specify a named volume and hostpath""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: /foo: hostpath: /bar name: foo-volume - """ - ) - invalid_config( - error_match="Volume /foo must have exactly one of 'hostpath' or 'name' subkey" + """, + error_match="Volume /foo must have exactly one of 'hostpath' or 'name' subkey", ) def test_complex_empty(self) -> None: """volumes complex form cannot be empty""" - SCUBA_YML.write_text( - r""" + invalid_config( + config_text=r""" image: na volumes: /foo: - """ - ) - invalid_config( - error_match="Volume /foo must have exactly one of 'hostpath' or 'name' subkey" + """, + error_match="Volume /foo must have exactly one of 'hostpath' or 'name' subkey", ) From 50b86765c3107e9aab860dc9cb101aca15e67945 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 19:38:59 -0400 Subject: [PATCH 10/25] tests: Use Path.write_text() in remaining cases in tests_config.py --- tests/test_config.py | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 224b2e8..aa9ff1d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -264,24 +264,27 @@ def test_load_config_image_from_yaml_missing_arg(self) -> None: GITLAB_YML.write_text("image: dummian:8.2") invalid_config(config_text=f"image: !from_yaml {GITLAB_YML}") - def __test_load_config_safe(self, bad_yaml_path) -> None: - with open(bad_yaml_path, "w") as f: - f.write("danger:\n") - f.write(" - !!python/object/apply:print [Danger]\n") - f.write(" - !!python/object/apply:sys.exit [66]\n") - + def __test_load_config_safe(self, bad_yaml_path: Path) -> None: + bad_yaml_path.write_text( + """ + danger: + - !!python/object/apply:print [Danger] + - !!python/object/apply:sys.exit [66] + """ + ) pat = "could not determine a constructor for the tag.*python/object/apply" with pytest.raises(scuba.config.ConfigError, match=pat) as ctx: load_config() def test_load_config_safe(self) -> None: """load_config safely loads yaml""" - self.__test_load_config_safe(".scuba.yml") + self.__test_load_config_safe(SCUBA_YML) def test_load_config_safe_external(self) -> None: """load_config safely loads yaml from external files""" - SCUBA_YML.write_text("image: !from_yaml .external.yml danger") - self.__test_load_config_safe(".external.yml") + external_yml = Path(".external.yml") + SCUBA_YML.write_text(f"image: !from_yaml {external_yml} danger") + self.__test_load_config_safe(external_yml) class TestConfigHooks(ConfigTest): @@ -651,16 +654,16 @@ def test_alias_docker_args_override_implicit_null(self) -> None: def test_alias_docker_args_override_from_yaml(self) -> None: """!override tag can be applied before a !from_yaml tag""" - with open("args.yml", "w") as f: - f.write("args: -v /tmp/:/tmp/\n") + args_yml = Path("args.yml") + args_yml.write_text("args: -v /tmp/:/tmp/") config = load_config( - config_text=r""" + config_text=rf""" image: na docker_args: --privileged aliases: testalias: - docker_args: !override '!from_yaml args.yml args' + docker_args: !override '!from_yaml {args_yml} args' script: - ugh """ @@ -672,16 +675,16 @@ def test_alias_docker_args_override_from_yaml(self) -> None: def test_alias_docker_args_from_yaml_override(self) -> None: """!override tag can be applied inside of a !from_yaml tag""" - with open("args.yml", "w") as f: - f.write("args: !override -v /tmp/:/tmp/\n") + args_yml = Path("args.yml") + args_yml.write_text("args: !override -v /tmp/:/tmp/") config = load_config( - config_text=r""" + config_text=rf""" image: na docker_args: --privileged aliases: testalias: - docker_args: !from_yaml args.yml args + docker_args: !from_yaml {args_yml} args script: - ugh """ From 653a604c7079d296786135577eb644fa239cb126 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 22:12:50 -0400 Subject: [PATCH 11/25] tests: Add missing PseudoTTY import --- tests/test_main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_main.py b/tests/test_main.py index 2aa5096..a8d0cb1 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -22,6 +22,7 @@ InTempDir, make_executable, skipUnlessTty, + PseudoTTY, ) From 035f275f12e64a6d862ed8e6cb2b320e4eaff560 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 22:58:35 -0400 Subject: [PATCH 12/25] tests: Add write_script() helper in test_main.py --- tests/test_main.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_main.py b/tests/test_main.py index a8d0cb1..6eecd8e 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -9,6 +9,7 @@ import subprocess import sys from tempfile import TemporaryFile, NamedTemporaryFile +from textwrap import dedent from unittest import mock import warnings @@ -30,6 +31,11 @@ SCUBAINIT_EXIT_FAIL = 99 +def write_script(path: Path, text: str) -> None: + path.write_text(dedent(text) + "\n") + make_executable(path) + + def run_scuba(args, *, expect_return=0, mock_isatty=False, stdin=None): """Run scuba, checking its return value From aecba5f1553f1b6c0ccc6af943f9a4e812db4259 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 22:17:31 -0400 Subject: [PATCH 13/25] tests: Use write_script() in TestMainBasic::test_args --- tests/test_main.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 6eecd8e..72f3e0e 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -194,15 +194,19 @@ def test_dry_run(self, subproc_call_mock): def test_args(self) -> None: """Verify scuba handles cmdline args""" SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") - - with open("test.sh", "w") as f: # TODO Path.write_text() - f.write("#!/bin/sh\n") - f.write('for a in "$@"; do echo $a; done\n') - make_executable("test.sh") + test_script = Path("test.sh") + + write_script( + test_script, + """\ + #!/bin/sh + for a in "$@"; do echo $a; done + """, + ) lines = ["here", "are", "some args"] - out, _ = run_scuba(["./test.sh"] + lines) + out, _ = run_scuba([f"./{test_script}"] + lines) assert_seq_equal(out.splitlines(), lines) From 51c98e40ac8e4b055a3af5edee0add83996631cc Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 22:18:11 -0400 Subject: [PATCH 14/25] tests: Use write_script() in TestMainStdinStdout These tests are currently unusable (skipped) because Pytest never runs with stdin connected to the actual tty (even when run locally). But this at leasts updates the tests to be consistent with others. Test: Locally removed isatty() assert and @skipUnlessTty() decorators and verified tests fail in an expected manner. --- tests/test_main.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 72f3e0e..019285c 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -223,19 +223,27 @@ def test_created_file_ownership(self) -> None: class TestMainStdinStdout(MainTest): + CHECK_TTY_SCRIPT = Path("check_tty.sh") + def _setup_test_tty(self) -> None: + assert sys.stdin.isatty() + SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") - with open("check_tty.sh", "w") as f: - f.write("#!/bin/sh\n") - f.write('if [ -t 1 ]; then echo "isatty"; else echo "notatty"; fi\n') - make_executable("check_tty.sh") + + write_script( + self.CHECK_TTY_SCRIPT, + """\ + #!/bin/sh + if [ -t 1 ]; then echo "isatty"; else echo "notatty"; fi + """, + ) @skipUnlessTty() def test_with_tty(self) -> None: """Verify docker allocates tty if stdout is a tty.""" self._setup_test_tty() - out, _ = run_scuba(["./check_tty.sh"], mock_isatty=True) + out, _ = run_scuba([f"./{self.CHECK_TTY_SCRIPT}"], mock_isatty=True) assert_str_equalish(out, "isatty") @@ -244,7 +252,7 @@ def test_without_tty(self) -> None: """Verify docker doesn't allocate tty if stdout is not a tty.""" self._setup_test_tty() - out, _ = run_scuba(["./check_tty.sh"]) + out, _ = run_scuba([f"./{self.CHECK_TTY_SCRIPT}"]) assert_str_equalish(out, "notatty") From 35f1ec4f99c4d798990bba3e12ff4fcde9e40d21 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 22:24:55 -0400 Subject: [PATCH 15/25] tests: Use Path.write_text() in TestMainAliasScripts --- tests/test_main.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 019285c..0bd2053 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -461,10 +461,12 @@ def mount_dummy(name): class TestMainAliasScripts(MainTest): def test_complex_commands_in_alias(self) -> None: """Verify complex commands can be used in alias scripts""" + test_dir = Path("foo") + test_file = test_dir / "bar.txt" test_string = "Hello world" - os.mkdir("foo") - with open("foo/bar.txt", "w") as f: - f.write(test_string) + + test_dir.mkdir() + test_file.write_text(test_string) SCUBA_YML.write_text( f""" @@ -472,7 +474,7 @@ def test_complex_commands_in_alias(self) -> None: aliases: alias1: script: - - cd foo && cat bar.txt + - cd {test_dir} && cat {test_file.name} """ ) From 2831357a8651b799792ebd5854e03dd431f1e804 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 23:01:48 -0400 Subject: [PATCH 16/25] tests: Use write_script() in TestMainEntrypoint --- tests/test_main.py | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 0bd2053..28c8564 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -541,16 +541,20 @@ def test_entrypoint_override(self) -> None: """ ) + test_script = Path("new.sh") test_str = "This is output from the overridden entrypoint" - with open("new.sh", "w") as f: - f.write("#!/bin/sh\n") - f.write(f'echo "{test_str}"\n') - make_executable("new.sh") + write_script( + test_script, + f"""\ + #!/bin/sh + echo "{test_str}" + """, + ) args = [ "--entrypoint", - os.path.abspath("new.sh"), + str(test_script.absolute()), "true", ] out, _ = run_scuba(args) @@ -581,24 +585,25 @@ def test_entrypoint_override_none(self) -> None: def test_yaml_entrypoint_override(self) -> None: """Verify entrypoint in .scuba.yml works""" + test_script = Path("new.sh") + test_str = "This is output from the overridden entrypoint" + + write_script( + test_script, + f"""\ + #!/bin/sh + echo "{test_str}" + """, + ) + SCUBA_YML.write_text( - """ + f""" image: scuba/entrypoint-test - entrypoint: "./new.sh" + entrypoint: "./{test_script}" """ ) - test_str = "This is output from the overridden entrypoint" - - with open("new.sh", "w") as f: - f.write("#!/bin/sh\n") - f.write(f'echo "{test_str}"\n') - make_executable("new.sh") - - args = [ - "true", - ] - out, _ = run_scuba(args) + out, _ = run_scuba(["true"]) assert_str_equalish(test_str, out) def test_yaml_entrypoint_override_none(self) -> None: From 27f2f722f81aff7fc651298d7346c235e35e7043 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 23:12:17 -0400 Subject: [PATCH 17/25] tests: Remove unnecessary "coding=utf-8" declaration See https://stackoverflow.com/a/14083123 --- tests/test_config.py | 1 - tests/test_dockerutil.py | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index aa9ff1d..d628913 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,4 +1,3 @@ -# coding=utf-8 import logging import os from os.path import join diff --git a/tests/test_dockerutil.py b/tests/test_dockerutil.py index dd418d4..23e3615 100644 --- a/tests/test_dockerutil.py +++ b/tests/test_dockerutil.py @@ -1,4 +1,3 @@ -# coding=utf-8 from pathlib import Path import pytest import subprocess From 602366f1576f6d2c22a0f763327b849fa6efd5bb Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 23:24:44 -0400 Subject: [PATCH 18/25] tests: Enable typing in test_utils.py --- tests/test_dockerutil.py | 2 +- tests/test_utils.py | 30 +++++++++++++++--------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/test_dockerutil.py b/tests/test_dockerutil.py index 23e3615..81545f0 100644 --- a/tests/test_dockerutil.py +++ b/tests/test_dockerutil.py @@ -33,7 +33,7 @@ def mocked_run(args, real_run=subprocess.run, **kw): uut.get_image_command("n/a") -def _test_get_images(stdout, returncode=0) -> Sequence[str]: +def _test_get_images(stdout: str, returncode: int = 0) -> Sequence[str]: def mocked_run(*args, **kwargs): mock_obj = mock.MagicMock() mock_obj.returncode = returncode diff --git a/tests/test_utils.py b/tests/test_utils.py index eef34f0..f9222b5 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -54,7 +54,7 @@ def test_format_cmdline() -> None: ) -def test_shell_quote_cmd(): +def test_shell_quote_cmd() -> None: args = ["foo", "bar pop", '"tee ball"'] result = scuba.utils.shell_quote_cmd(args) @@ -64,44 +64,44 @@ def test_shell_quote_cmd(): assert_seq_equal(out_args, args) -def test_parse_env_var(): +def test_parse_env_var() -> None: """parse_env_var returns a key, value pair""" result = scuba.utils.parse_env_var("KEY=value") assert result == ("KEY", "value") -def test_parse_env_var_more_equals(): +def test_parse_env_var_more_equals() -> None: """parse_env_var handles multiple equals signs""" result = scuba.utils.parse_env_var("KEY=anotherkey=value") assert result == ("KEY", "anotherkey=value") -def test_parse_env_var_no_equals(monkeypatch): +def test_parse_env_var_no_equals(monkeypatch: pytest.MonkeyPatch) -> None: """parse_env_var handles no equals and gets value from environment""" monkeypatch.setenv("KEY", "mockedvalue") result = scuba.utils.parse_env_var("KEY") assert result == ("KEY", "mockedvalue") -def test_parse_env_var_not_set(monkeypatch): +def test_parse_env_var_not_set(monkeypatch: pytest.MonkeyPatch) -> None: """parse_env_var returns an empty string if not set""" monkeypatch.delenv("NOTSET", raising=False) result = scuba.utils.parse_env_var("NOTSET") assert result == ("NOTSET", "") -def test_flatten_list__not_list(): +def test_flatten_list__not_list() -> None: with pytest.raises(ValueError): - scuba.utils.flatten_list("abc") + scuba.utils.flatten_list("abc") # type: ignore[arg-type] -def test_flatten_list__not_nested(): +def test_flatten_list__not_nested() -> None: sample = [1, 2, 3, 4] result = scuba.utils.flatten_list(sample) assert result == sample -def test_flatten_list__nested_1(): +def test_flatten_list__nested_1() -> None: sample = [ 1, [2, 3], @@ -113,7 +113,7 @@ def test_flatten_list__nested_1(): assert_seq_equal(result, exp) -def test_flatten_list__nested_many(): +def test_flatten_list__nested_many() -> None: sample = [ 1, [2, 3], @@ -127,7 +127,7 @@ def test_flatten_list__nested_many(): assert_seq_equal(result, exp) -def test_get_umask(): +def test_get_umask() -> None: testval = 0o123 # unlikely default orig = os.umask(testval) try: @@ -143,14 +143,14 @@ def test_get_umask(): os.umask(orig) -def test_writeln(): +def test_writeln() -> None: with io.StringIO() as s: scuba.utils.writeln(s, "hello") scuba.utils.writeln(s, "goodbye") assert s.getvalue() == "hello\ngoodbye\n" -def test_expand_env_vars(monkeypatch): +def test_expand_env_vars(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("MY_VAR", "my favorite variable") assert ( scuba.utils.expand_env_vars("This is $MY_VAR") == "This is my favorite variable" @@ -161,7 +161,7 @@ def test_expand_env_vars(monkeypatch): ) -def test_expand_missing_env_vars(monkeypatch): +def test_expand_missing_env_vars(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("MY_VAR", raising=False) # Verify that a KeyError is raised for unset env variables with pytest.raises(KeyError) as kerr: @@ -169,7 +169,7 @@ def test_expand_missing_env_vars(monkeypatch): assert kerr.value.args[0] == "MY_VAR" -def test_expand_env_vars_dollars(): +def test_expand_env_vars_dollars() -> None: # Verify that a ValueError is raised for bare, unescaped '$' characters with pytest.raises(ValueError): scuba.utils.expand_env_vars("Just a lonely $") From 2ccf6718b5f7ed8c83195fe461c80b4fe16f4376 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 23:44:04 -0400 Subject: [PATCH 19/25] tests: Improve typing in test_config.py --- tests/conftest.py | 3 ++- tests/test_config.py | 38 ++++++++++++++++++++++---------------- tests/utils.py | 4 ++-- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index dd4793e..3483ffd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,8 +1,9 @@ +from pathlib import Path import pytest @pytest.fixture -def in_tmp_path(tmp_path, monkeypatch): +def in_tmp_path(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: """Runs a test in a temporary directory provided by the tmp_path fixture""" monkeypatch.chdir(tmp_path) return tmp_path diff --git a/tests/test_config.py b/tests/test_config.py index d628913..9b2e7f3 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -77,7 +77,7 @@ class ConfigTest: class TestFindConfig(ConfigTest): - def test_find_config_cur_dir(self, in_tmp_path) -> None: + def test_find_config_cur_dir(self, in_tmp_path: Path) -> None: """find_config can find the config in the current directory""" SCUBA_YML.write_text("image: bosybux") @@ -85,7 +85,7 @@ def test_find_config_cur_dir(self, in_tmp_path) -> None: assert_paths_equal(path, in_tmp_path) assert_paths_equal(rel, "") - def test_find_config_parent_dir(self, in_tmp_path) -> None: + def test_find_config_parent_dir(self, in_tmp_path: Path) -> None: """find_config cuba can find the config in the parent directory""" SCUBA_YML.write_text("image: bosybux") @@ -99,13 +99,13 @@ def test_find_config_parent_dir(self, in_tmp_path) -> None: assert_paths_equal(path, in_tmp_path) assert_paths_equal(rel, "subdir") - def test_find_config_way_up(self, in_tmp_path) -> None: + def test_find_config_way_up(self, in_tmp_path: Path) -> None: """find_config can find the config way up the directory hierarchy""" SCUBA_YML.write_text("image: bosybux") subdirs = ["foo", "bar", "snap", "crackle", "pop"] - for sd in subdirs: + for sd in subdirs: # TODO os.mkdir(sd) os.chdir(sd) @@ -341,7 +341,7 @@ def test_env_invalid(self) -> None: error_match="must be list or mapping", ) - def test_env_top_dict(self, monkeypatch) -> None: + def test_env_top_dict(self, monkeypatch: pytest.MonkeyPatch) -> None: """Top-level environment can be loaded (dict)""" monkeypatch.setenv("EXTERNAL", "Outside world") monkeypatch.delenv("EXTERNAL_NOTSET", raising=False) @@ -373,7 +373,7 @@ def test_env_top_dict(self, monkeypatch) -> None: ) assert expect == config.environment - def test_env_top_list(self, monkeypatch) -> None: + def test_env_top_list(self, monkeypatch: pytest.MonkeyPatch) -> None: """Top-level environment can be loaded (list)""" monkeypatch.setenv("EXTERNAL", "Outside world") monkeypatch.delenv("EXTERNAL_NOTSET", raising=False) @@ -836,7 +836,7 @@ def test_via_alias(self) -> None: assert_vol(vols, "/foo", "/host/foo") assert_vol(vols, "/bar", "/host/bar", ["z", "ro"]) - def test_with_env_vars_simple(self, monkeypatch) -> None: + def test_with_env_vars_simple(self, monkeypatch: pytest.MonkeyPatch) -> None: """volume definitions can contain environment variables""" monkeypatch.setenv("TEST_VOL_PATH", "/bar/baz") monkeypatch.setenv("TEST_VOL_PATH2", "/moo/doo") @@ -853,7 +853,7 @@ def test_with_env_vars_simple(self, monkeypatch) -> None: assert_vol(vols, "/bar/baz/foo", "/moo/doo/foo") - def test_with_env_vars_complex(self, monkeypatch) -> None: + def test_with_env_vars_complex(self, monkeypatch: pytest.MonkeyPatch) -> None: """complex volume definitions can contain environment variables""" monkeypatch.setenv("TEST_HOME", "/home/testuser") monkeypatch.setenv("TEST_TMP", "/tmp") @@ -881,7 +881,7 @@ def test_with_env_vars_complex(self, monkeypatch) -> None: vols, "/var/spool/mail/container", "/var/spool/mail/testuser", ["z", "ro"] ) - def test_with_invalid_env_vars(self, monkeypatch) -> None: + def test_with_invalid_env_vars(self, monkeypatch: pytest.MonkeyPatch) -> None: """Volume definitions cannot include unset env vars""" # Ensure that the entry does not exist in the environment monkeypatch.delenv("TEST_VAR1", raising=False) @@ -894,7 +894,9 @@ def test_with_invalid_env_vars(self, monkeypatch) -> None: error_match="TEST_VAR1", ) - def test_hostpath_rel(self, monkeypatch, in_tmp_path) -> None: + def test_hostpath_rel( + self, monkeypatch: pytest.MonkeyPatch, in_tmp_path: Path + ) -> None: """volume hostpath can be relative to scuba_root (top dir)""" monkeypatch.setenv("RELVAR", "./spam/eggs") @@ -924,7 +926,9 @@ def test_hostpath_rel(self, monkeypatch, in_tmp_path) -> None: assert_vol(config.volumes, "/scp", in_tmp_path / "snap" / "crackle" / "pop") assert_vol(config.volumes, "/relvar", in_tmp_path / "spam" / "eggs") - def test_hostpath_rel_above(self, monkeypatch, in_tmp_path) -> None: + def test_hostpath_rel_above( + self, monkeypatch: pytest.MonkeyPatch, in_tmp_path: Path + ) -> None: """volume hostpath can be relative, above scuba_root (top dir)""" # Directory structure: # @@ -957,7 +961,7 @@ def test_hostpath_rel_above(self, monkeypatch, in_tmp_path) -> None: assert config.volumes is not None assert_vol(config.volumes, "/foo", in_tmp_path / "foo_up_here") - def test_hostpath_rel_requires_dot_complex(self, monkeypatch, in_tmp_path) -> None: + def test_hostpath_rel_requires_dot_complex(self) -> None: """relaitve volume hostpath (complex form) must start with ./ or ../""" invalid_config( config_text=r""" @@ -969,7 +973,9 @@ def test_hostpath_rel_requires_dot_complex(self, monkeypatch, in_tmp_path) -> No error_match="Relative path must start with ./ or ../", ) - def test_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: + def test_hostpath_rel_in_env( + self, monkeypatch: pytest.MonkeyPatch, in_tmp_path: Path + ) -> None: """volume definitions can contain environment variables, including relative path portions""" monkeypatch.setenv("PREFIX", "./") config = load_config( @@ -985,7 +991,7 @@ def test_hostpath_rel_in_env(self, monkeypatch, in_tmp_path) -> None: assert_vol(vols, "/foo", in_tmp_path / "foo") - def test_contpath_rel(self, monkeypatch, in_tmp_path) -> None: + def test_contpath_rel(self) -> None: invalid_config( config_text=r""" image: na @@ -1012,7 +1018,7 @@ def test_simple_named_volume(self) -> None: assert_paths_equal(vol.container_path, "/foo") assert vol.volume_name == "foo-volume" - def test_simple_named_volume_env(self, monkeypatch) -> None: + def test_simple_named_volume_env(self, monkeypatch: pytest.MonkeyPatch) -> None: """volumes simple form can specify a named volume via env var""" monkeypatch.setenv("FOO_VOLUME", "foo-volume") config = load_config( @@ -1030,7 +1036,7 @@ def test_simple_named_volume_env(self, monkeypatch) -> None: assert_paths_equal(vol.container_path, "/foo") assert vol.volume_name == "foo-volume" - def test_complex_named_volume_env(self, monkeypatch) -> None: + def test_complex_named_volume_env(self, monkeypatch: pytest.MonkeyPatch) -> None: """volumes complex form can specify a named volume via env var""" monkeypatch.setenv("FOO_VOLUME", "foo-volume") config = load_config( diff --git a/tests/utils.py b/tests/utils.py index 8e78fea..7e7e468 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -39,8 +39,8 @@ def assert_str_equalish(exp: Any, act: Any) -> None: def assert_vol( vols: Dict[Path, ScubaVolume], - cpath_str: str, - hpath_str: str, + cpath_str: PathStr, + hpath_str: PathStr, options: List[str] = [], ) -> None: cpath = Path(cpath_str) From 2f5c4060900a939f37b6ba976672b9ef754b446b Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Sun, 24 Mar 2024 23:54:00 -0400 Subject: [PATCH 20/25] tests: Add type annotations in utils.py --- tests/utils.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 7e7e468..cfb7b91 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,3 +1,4 @@ +from __future__ import annotations import os import sys from os.path import normpath @@ -6,13 +7,15 @@ import unittest import logging from pathlib import Path -from typing import Any, Dict, List, Sequence, Union +from typing import Any, Callable, Dict, List, Sequence, TypeVar, Optional, Union from unittest import mock from scuba.config import ScubaVolume PathStr = Union[Path, str] +_FT = TypeVar("_FT", bound=Callable[..., Any]) + def assert_seq_equal(a: Sequence, b: Sequence) -> None: assert list(a) == list(b) @@ -62,33 +65,39 @@ def make_executable(path: PathStr) -> None: # http://stackoverflow.com/a/8389373/119527 class PseudoTTY: - def __init__(self, underlying): + def __init__(self, underlying: object): self.__underlying = underlying - def __getattr__(self, name): + def __getattr__(self, name: str) -> Any: return getattr(self.__underlying, name) - def isatty(self): + def isatty(self) -> bool: return True -def skipUnlessTty(): +def skipUnlessTty() -> Callable[[_FT], _FT]: return unittest.skipUnless( sys.stdin.isatty(), "Can't test docker tty if not connected to a terminal" ) class InTempDir: - def __init__(self, suffix="", prefix="tmp", dir=None, delete=True): + def __init__( + self, + suffix: str = "", + prefix: str = "tmp", + dir: Optional[PathStr] = None, + delete: bool = True, + ): self.delete = delete self.temp_path = tempfile.mkdtemp(suffix=suffix, prefix=prefix, dir=dir) - def __enter__(self): + def __enter__(self) -> InTempDir: self.orig_path = os.getcwd() os.chdir(self.temp_path) return self - def __exit__(self, *exc_info): + def __exit__(self, *exc_info: Any) -> None: # Restore the working dir and cleanup the temp one os.chdir(self.orig_path) if self.delete: From 3635f87d9cf8ca2b6f138d8d1e828f48fe962faf Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Mon, 25 Mar 2024 00:27:35 -0400 Subject: [PATCH 21/25] tests: Improve typing in test_main.py --- tests/test_main.py | 115 +++++++++++++++++++++++++-------------------- 1 file changed, 63 insertions(+), 52 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index 28c8564..e4437e4 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -10,6 +10,7 @@ import sys from tempfile import TemporaryFile, NamedTemporaryFile from textwrap import dedent +from typing import cast, IO, List, Optional, Sequence, TextIO, Tuple from unittest import mock import warnings @@ -26,6 +27,8 @@ PseudoTTY, ) +ScubaResult = Tuple[str, str] + SCUBA_YML = Path(".scuba.yml") SCUBAINIT_EXIT_FAIL = 99 @@ -36,7 +39,13 @@ def write_script(path: Path, text: str) -> None: make_executable(path) -def run_scuba(args, *, expect_return=0, mock_isatty=False, stdin=None): +def run_scuba( + args: List[str], + *, + expect_return: int = 0, + mock_isatty: bool = False, + stdin: Optional[IO[str]] = None, +) -> ScubaResult: """Run scuba, checking its return value Returns scuba/docker stdout data. @@ -47,8 +56,8 @@ def run_scuba(args, *, expect_return=0, mock_isatty=False, stdin=None): with TemporaryFile(prefix="scubatest-stdout", mode="w+t") as stdout: with TemporaryFile(prefix="scubatest-stderr", mode="w+t") as stderr: if mock_isatty: - stdout = PseudoTTY(stdout) - stderr = PseudoTTY(stderr) + stdout = PseudoTTY(stdout) # type: ignore[assignment] + stderr = PseudoTTY(stderr) # type: ignore[assignment] old_stdin = sys.stdin old_stdout = sys.stdout @@ -57,9 +66,9 @@ def run_scuba(args, *, expect_return=0, mock_isatty=False, stdin=None): if stdin is None: sys.stdin = open(os.devnull, "w") else: - sys.stdin = stdin - sys.stdout = stdout - sys.stderr = stderr + sys.stdin = cast(TextIO, stdin) + sys.stdout = cast(TextIO, stdout) + sys.stderr = cast(TextIO, stderr) try: """ @@ -127,7 +136,7 @@ def test_handle_get_image_command_error(self) -> None: """Verify scuba handles a get_image_command error""" SCUBA_YML.write_text("image: {DOCKER_IMAGE}") - def mocked_gic(*args, **kw): + def mocked_gic(image: str) -> Optional[Sequence[str]]: raise scuba.dockerutil.DockerError("mock error") # http://alexmarandon.com/articles/python_mock_gotchas/#patching-in-the-wrong-place @@ -169,7 +178,7 @@ def test_version(self) -> None: assert name == "scuba" assert ver == scuba.__version__ - def test_no_docker(self, monkeypatch) -> None: + def test_no_docker(self, monkeypatch: pytest.MonkeyPatch) -> None: """Verify scuba gracefully handles docker not being installed""" SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") @@ -179,12 +188,12 @@ def test_no_docker(self, monkeypatch) -> None: _, err = run_scuba(args, expect_return=2) @mock.patch("subprocess.call") - def test_dry_run(self, subproc_call_mock): + def test_dry_run(self, subproc_call_mock: mock.Mock) -> None: + print(f"subproc_call_mock is a {type(subproc_call_mock)}") """Verify scuba handles --dry-run and --verbose""" SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = ["--dry-run", "--verbose", "/bin/false"] - _, err = run_scuba(args) assert not subproc_call_mock.called @@ -272,12 +281,12 @@ def test_redirect_stdin(self) -> None: class TestMainUser(MainTest): def _test_user( self, - expected_uid, - expected_username, - expected_gid, - expected_groupname, - scuba_args=[], - ): + expected_uid: int, + expected_username: str, + expected_gid: int, + expected_groupname: str, + scuba_args: List[str] = [], + ) -> None: SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = scuba_args + [ @@ -287,15 +296,24 @@ def _test_user( ] out, _ = run_scuba(args) - uid, username, gid, groupname = out.split() - uid = int(uid) - gid = int(gid) + uid_str, username, gid_str, groupname = out.split() + uid = int(uid_str) + gid = int(gid_str) assert uid == expected_uid assert username == expected_username assert gid == expected_gid assert groupname == expected_groupname + def _test_user_expect_root(self, scuba_args: List[str] = []) -> None: + return self._test_user( + expected_uid=0, + expected_username="root", + expected_gid=0, + expected_groupname="root", + scuba_args=scuba_args, + ) + def test_user_scubauser(self) -> None: """Verify scuba runs container as the current (host) uid/gid""" self._test_user( @@ -305,19 +323,9 @@ def test_user_scubauser(self) -> None: expected_groupname=getgrgid(os.getgid()).gr_name, ) - EXPECT_ROOT = dict( - expected_uid=0, - expected_username="root", - expected_gid=0, - expected_groupname="root", - ) - def test_user_root(self) -> None: """Verify scuba -r runs container as root""" - self._test_user( - **self.EXPECT_ROOT, - scuba_args=["-r"], - ) + self._test_user_expect_root(scuba_args=["-r"]) def test_user_run_as_root(self) -> None: '''Verify running scuba as root is identical to "scuba -r"''' @@ -325,7 +333,7 @@ def test_user_run_as_root(self) -> None: with mock.patch("os.getuid", return_value=0) as getuid_mock, mock.patch( "os.getgid", return_value=0 ) as getgid_mock: - self._test_user(**self.EXPECT_ROOT) + self._test_user_expect_root() assert getuid_mock.called assert getgid_mock.called @@ -373,7 +381,7 @@ def test_user_root_alias(self) -> None: class TestMainHomedir(MainTest): - def _test_home_writable(self, scuba_args=[]): + def _test_home_writable(self, scuba_args: List[str] = []) -> None: SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = scuba_args + [ @@ -435,7 +443,7 @@ def test_arbitrary_docker_args_merge_config(self) -> None: expfiles = set() tgtdir = "/tgtdir" - def mount_dummy(name): + def mount_dummy(name: str) -> str: assert name not in expfiles expfiles.add(name) return f'-v "{dummy.absolute()}:{tgtdir}/{name}"\n' @@ -686,7 +694,13 @@ def test_yml_not_needed_with_image_override(self) -> None: class TestMainHooks(MainTest): - def _test_one_hook(self, hookname, hookcmd, cmd, expect_return=0): + def _test_one_hook( + self, + hookname: str, + hookcmd: str, + cmd: str, + expect_return: int = 0, + ) -> ScubaResult: SCUBA_YML.write_text( f""" image: {DOCKER_IMAGE} @@ -698,19 +712,19 @@ def _test_one_hook(self, hookname, hookcmd, cmd, expect_return=0): args = ["/bin/sh", "-c", cmd] return run_scuba(args, expect_return=expect_return) - def _test_hook_runs_as(self, hookname, exp_uid, exp_gid) -> None: + def _test_hook_runs_as(self, hookname: str, exp_uid: int, exp_gid: int) -> None: out, _ = self._test_one_hook( - hookname, - "echo $(id -u) $(id -g)", - "echo success", + hookname=hookname, + hookcmd="echo $(id -u) $(id -g)", + cmd="echo success", ) - out = out.splitlines() + out_lines = out.splitlines() - uid, gid = map(int, out[0].split()) + uid, gid = map(int, out_lines[0].split()) assert exp_uid == uid assert exp_gid == gid - assert_str_equalish(out[1], "success") + assert_str_equalish(out_lines[1], "success") def test_user_hook_runs_as_user(self) -> None: """Verify user hook executes as user""" @@ -745,7 +759,7 @@ def test_env_var_keyval(self) -> None: out, _ = run_scuba(args) assert_str_equalish(out, "VAL") - def test_env_var_key_only(self, monkeypatch): + def test_env_var_key_only(self, monkeypatch: pytest.MonkeyPatch) -> None: """Verify -e KEY works""" SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") args = [ @@ -759,7 +773,7 @@ def test_env_var_key_only(self, monkeypatch): out, _ = run_scuba(args) assert_str_equalish(out, "mockedvalue") - def test_env_var_sources(self, monkeypatch): + def test_env_var_sources(self, monkeypatch: pytest.MonkeyPatch) -> None: """Verify scuba handles all possible environment variable sources""" SCUBA_YML.write_text( rf""" @@ -812,7 +826,7 @@ def test_env_var_sources(self, monkeypatch): BAZ="From the command line", ) - def test_builtin_env__SCUBA_ROOT(self, in_tmp_path): + def test_builtin_env__SCUBA_ROOT(self, in_tmp_path: Path) -> None: """Verify SCUBA_ROOT is set in container""" SCUBA_YML.write_text(f"image: {DOCKER_IMAGE}") @@ -945,8 +959,7 @@ def test_volumes_basic(self) -> None: ) out, _ = run_scuba(["doit"]) - out = out.splitlines() - assert out == ["from the top", "from the alias"] + assert out.splitlines() == ["from the top", "from the alias"] def test_volumes_alias_override(self) -> None: """Verify volumes can be overridden by an alias""" @@ -975,13 +988,11 @@ def test_volumes_alias_override(self) -> None: # Run a non-alias command out, _ = run_scuba(["cat", "/data/thing"]) - out = out.splitlines() - assert out == ["from the top"] + assert out.splitlines() == ["from the top"] # Run the alias out, _ = run_scuba(["doit"]) - out = out.splitlines() - assert out == ["from the alias"] + assert out.splitlines() == ["from the alias"] def test_volumes_host_path_create(self) -> None: """Missing host paths should be created before starting Docker""" @@ -1136,10 +1147,10 @@ def _rm_volume(self) -> None: return result.check_returncode() - def setup_method(self, method) -> None: + def setup_method(self) -> None: self._rm_volume() - def teardown_method(self, method) -> None: + def teardown_method(self) -> None: self._rm_volume() def test_volumes_named(self) -> None: From dc7b8e70507212f1205fd1358eed3039a45b4427 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Mon, 25 Mar 2024 00:37:25 -0400 Subject: [PATCH 22/25] tests: Ignore untyped mock.patch(side_effect=) functions These are just not worth the effort to properly annotate! --- tests/test_dockerutil.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_dockerutil.py b/tests/test_dockerutil.py index 81545f0..2dac1e1 100644 --- a/tests/test_dockerutil.py +++ b/tests/test_dockerutil.py @@ -23,7 +23,7 @@ def test_get_image_command_bad_image() -> None: def test_get_image_no_docker() -> None: """get_image_command raises an exception if docker is not installed""" - def mocked_run(args, real_run=subprocess.run, **kw): + def mocked_run(args, real_run=subprocess.run, **kw): # type: ignore[no-untyped-def] assert args[0] == "docker" args[0] = "dockerZZZZ" return real_run(args, **kw) @@ -34,7 +34,7 @@ def mocked_run(args, real_run=subprocess.run, **kw): def _test_get_images(stdout: str, returncode: int = 0) -> Sequence[str]: - def mocked_run(*args, **kwargs): + def mocked_run(*args, **kwargs): # type: ignore[no-untyped-def] mock_obj = mock.MagicMock() mock_obj.returncode = returncode mock_obj.stdout = stdout From 7d4de93383d42f95e832d8c9407a6d31a55b1afd Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Mon, 25 Mar 2024 00:38:55 -0400 Subject: [PATCH 23/25] mypy: Disallow untyped/incomplete defs in all Python code This previously applied only to the scuba package but now applies to tests as well. --- pyproject.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 0c3a835..99ef4d9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,9 +54,6 @@ requires = [ packages = ["scuba", "tests"] warn_unused_configs = true warn_return_any = true - -[[tool.mypy.overrides]] -module = "scuba.*" disallow_untyped_calls = true disallow_untyped_defs = true disallow_incomplete_defs = true From e656eb447574dfe3be1a82cc534b1f0fe8918f23 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Mon, 25 Mar 2024 00:49:50 -0400 Subject: [PATCH 24/25] tests: Correct inappropriate all caps local vars --- tests/test_main.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_main.py b/tests/test_main.py index e4437e4..455c52a 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1155,23 +1155,23 @@ def teardown_method(self) -> None: def test_volumes_named(self) -> None: """Verify named volumes can be used""" - VOL_PATH = Path("/foo") - TEST_PATH = VOL_PATH / "test.txt" - TEST_STR = "it works!" + vol_path = Path("/foo") + test_path = vol_path / "test.txt" + test_str = "it works!" SCUBA_YML.write_text( f""" image: {DOCKER_IMAGE} hooks: - root: chmod 777 {VOL_PATH} + root: chmod 777 {vol_path} volumes: - {VOL_PATH}: {self.VOLUME_NAME} + {vol_path}: {self.VOLUME_NAME} """ ) # Inoke scuba once: Write a file to the named volume - run_scuba(["/bin/sh", "-c", f"echo {TEST_STR} > {TEST_PATH}"]) + run_scuba(["/bin/sh", "-c", f"echo {test_str} > {test_path}"]) # Invoke scuba again: Verify the file is still there - out, _ = run_scuba(["/bin/sh", "-c", f"cat {TEST_PATH}"]) - assert_str_equalish(out, TEST_STR) + out, _ = run_scuba(["/bin/sh", "-c", f"cat {test_path}"]) + assert_str_equalish(out, test_str) From fe16200a840aa0f9b4ed5d9c25a761384b93e5f8 Mon Sep 17 00:00:00 2001 From: Jonathon Reinhart Date: Mon, 25 Mar 2024 00:58:05 -0400 Subject: [PATCH 25/25] tests: Simplify TestFindConfig with use of Path --- tests/test_config.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 9b2e7f3..95b255e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,9 +1,7 @@ import logging import os -from os.path import join from pathlib import Path import pytest -from shutil import rmtree from typing import Optional from unittest import mock @@ -89,32 +87,31 @@ def test_find_config_parent_dir(self, in_tmp_path: Path) -> None: """find_config cuba can find the config in the parent directory""" SCUBA_YML.write_text("image: bosybux") - os.mkdir("subdir") - os.chdir("subdir") + subdir = Path("subdir") + subdir.mkdir() + os.chdir(subdir) # Verify our current working dir - assert_paths_equal(os.getcwd(), in_tmp_path.joinpath("subdir")) + assert_paths_equal(Path.cwd(), in_tmp_path / subdir) path, rel, _ = scuba.config.find_config() assert_paths_equal(path, in_tmp_path) - assert_paths_equal(rel, "subdir") + assert_paths_equal(rel, subdir) def test_find_config_way_up(self, in_tmp_path: Path) -> None: """find_config can find the config way up the directory hierarchy""" SCUBA_YML.write_text("image: bosybux") - subdirs = ["foo", "bar", "snap", "crackle", "pop"] - - for sd in subdirs: # TODO - os.mkdir(sd) - os.chdir(sd) + subdir = Path("foo/bar/snap/crackle/pop") + subdir.mkdir(parents=True) + os.chdir(subdir) # Verify our current working dir - assert_paths_equal(os.getcwd(), in_tmp_path.joinpath(*subdirs)) + assert_paths_equal(Path.cwd(), in_tmp_path / subdir) path, rel, _ = scuba.config.find_config() assert_paths_equal(path, in_tmp_path) - assert_paths_equal(rel, join(*subdirs)) + assert_paths_equal(rel, subdir) def test_find_config_nonexist(self) -> None: """find_config raises ConfigError if the config cannot be found"""