From da0bd2d56e962debb80fedbb22f133df879302a1 Mon Sep 17 00:00:00 2001 From: David Murphy Date: Wed, 14 Aug 2024 13:49:11 -0600 Subject: [PATCH 1/4] Removing tech debt and restoring original pytest.skip for tests --- tests/pytests/pkg/integration/test_enabled_disabled.py | 5 +---- tests/pytests/pkg/integration/test_salt_api.py | 2 +- tests/pytests/pkg/integration/test_salt_call.py | 4 ---- tests/pytests/pkg/integration/test_salt_exec.py | 2 +- tests/pytests/pkg/integration/test_salt_grains.py | 2 +- tests/pytests/pkg/integration/test_salt_key.py | 2 +- tests/pytests/pkg/integration/test_salt_output.py | 2 +- tests/pytests/pkg/integration/test_salt_state_file.py | 2 +- tests/pytests/pkg/integration/test_salt_user.py | 5 ++++- tests/pytests/pkg/integration/test_version.py | 9 +++++---- 10 files changed, 16 insertions(+), 19 deletions(-) diff --git a/tests/pytests/pkg/integration/test_enabled_disabled.py b/tests/pytests/pkg/integration/test_enabled_disabled.py index 9b98d47becd4..43dd4d4366a0 100644 --- a/tests/pytests/pkg/integration/test_enabled_disabled.py +++ b/tests/pytests/pkg/integration/test_enabled_disabled.py @@ -1,11 +1,8 @@ import pytest from pytestskipmarkers.utils import platform -pytestmark = [ - pytest.mark.skip_unless_on_linux, -] - +@pytest.mark.skip_on_windows(reason="Linux test only") def test_services(install_salt, salt_call_cli): """ Check if Services are enabled/disabled diff --git a/tests/pytests/pkg/integration/test_salt_api.py b/tests/pytests/pkg/integration/test_salt_api.py index 8706ed9e6305..e962fbe32213 100644 --- a/tests/pytests/pkg/integration/test_salt_api.py +++ b/tests/pytests/pkg/integration/test_salt_api.py @@ -1,7 +1,7 @@ import pytest pytestmark = [ - pytest.mark.skip_unless_on_linux, + pytest.mark.skip_on_windows, ] diff --git a/tests/pytests/pkg/integration/test_salt_call.py b/tests/pytests/pkg/integration/test_salt_call.py index fe3bc1728aa3..c16ecb67481d 100644 --- a/tests/pytests/pkg/integration/test_salt_call.py +++ b/tests/pytests/pkg/integration/test_salt_call.py @@ -3,10 +3,6 @@ import pytest from pytestskipmarkers.utils import platform -pytestmark = [ - pytest.mark.skip_unless_on_linux, -] - def test_salt_call_local(salt_call_cli): """ diff --git a/tests/pytests/pkg/integration/test_salt_exec.py b/tests/pytests/pkg/integration/test_salt_exec.py index cad14b6ba026..2e28999d7c3e 100644 --- a/tests/pytests/pkg/integration/test_salt_exec.py +++ b/tests/pytests/pkg/integration/test_salt_exec.py @@ -3,7 +3,7 @@ import pytest pytestmark = [ - pytest.mark.skip_unless_on_linux, + pytest.mark.skip_on_windows, ] diff --git a/tests/pytests/pkg/integration/test_salt_grains.py b/tests/pytests/pkg/integration/test_salt_grains.py index d8da338ec2cf..2a609cb9ea04 100644 --- a/tests/pytests/pkg/integration/test_salt_grains.py +++ b/tests/pytests/pkg/integration/test_salt_grains.py @@ -1,7 +1,7 @@ import pytest pytestmark = [ - pytest.mark.skip_unless_on_linux, + pytest.mark.skip_on_windows, ] diff --git a/tests/pytests/pkg/integration/test_salt_key.py b/tests/pytests/pkg/integration/test_salt_key.py index 4e8fb33ff7ca..87275a677fa1 100644 --- a/tests/pytests/pkg/integration/test_salt_key.py +++ b/tests/pytests/pkg/integration/test_salt_key.py @@ -1,7 +1,7 @@ import pytest pytestmark = [ - pytest.mark.skip_unless_on_linux, + pytest.mark.skip_on_windows, ] diff --git a/tests/pytests/pkg/integration/test_salt_output.py b/tests/pytests/pkg/integration/test_salt_output.py index 3c008774473d..b4d61044846d 100644 --- a/tests/pytests/pkg/integration/test_salt_output.py +++ b/tests/pytests/pkg/integration/test_salt_output.py @@ -1,7 +1,7 @@ import pytest pytestmark = [ - pytest.mark.skip_unless_on_linux, + pytest.mark.skip_on_windows, ] diff --git a/tests/pytests/pkg/integration/test_salt_state_file.py b/tests/pytests/pkg/integration/test_salt_state_file.py index 8e6c98b6fc02..0c4804654cb8 100644 --- a/tests/pytests/pkg/integration/test_salt_state_file.py +++ b/tests/pytests/pkg/integration/test_salt_state_file.py @@ -6,7 +6,7 @@ from saltfactories.utils.functional import MultiStateResult pytestmark = [ - pytest.mark.skip_unless_on_linux, + pytest.mark.skip_on_windows, ] diff --git a/tests/pytests/pkg/integration/test_salt_user.py b/tests/pytests/pkg/integration/test_salt_user.py index 310d283f1139..280b51624e2d 100644 --- a/tests/pytests/pkg/integration/test_salt_user.py +++ b/tests/pytests/pkg/integration/test_salt_user.py @@ -8,7 +8,10 @@ import pytest from saltfactories.utils.tempfiles import temp_directory -pytestmark = [pytest.mark.skip_unless_on_linux] +pytestmark = [ + pytest.mark.skip_on_windows, + pytest.mark.skip_on_darwin, +] @pytest.fixture diff --git a/tests/pytests/pkg/integration/test_version.py b/tests/pytests/pkg/integration/test_version.py index 48cf70817021..cda029211087 100644 --- a/tests/pytests/pkg/integration/test_version.py +++ b/tests/pytests/pkg/integration/test_version.py @@ -5,11 +5,8 @@ import pytest from pytestskipmarkers.utils import platform -pytestmark = [ - pytest.mark.skip_unless_on_linux, -] - +@pytest.mark.skip_on_windows def test_salt_version(version, install_salt): """ Test version output from salt --version @@ -37,6 +34,7 @@ def test_salt_version(version, install_salt): assert actual == expected +@pytest.mark.skip_on_windows def test_salt_versions_report_master(install_salt): """ Test running --versions-report on master @@ -57,6 +55,7 @@ def test_salt_versions_report_master(install_salt): ret.stdout.matcher.fnmatch_lines([f"*{py_version}*"]) +@pytest.mark.skip_on_windows def test_salt_versions_report_minion(salt_cli, salt_call_cli, salt_minion): """ Test running test.versions_report on minion @@ -140,6 +139,8 @@ def test_symlinks_created(version, symlink, install_salt): ret.stdout.matcher.fnmatch_lines([f"*{version}*"]) +@pytest.mark.skip_on_windows +@pytest.mark.skip_on_darwin def test_compare_pkg_versions_redhat_rc(version, install_salt): """ Test compare pkg versions for redhat RC packages. A tilde should be included From a2cb7b9937863f3c5a676950b151c8082efc1dc5 Mon Sep 17 00:00:00 2001 From: David Murphy Date: Wed, 14 Aug 2024 14:05:43 -0600 Subject: [PATCH 2/4] Adjusted tests for classic or relenv, as everything is relenv now --- tests/pytests/pkg/conftest.py | 28 ++----------------- .../pkg/downgrade/test_salt_downgrade.py | 2 -- .../integration/test_clean_zmq_teardown.py | 6 ---- tests/pytests/pkg/integration/test_pip.py | 8 ------ tests/pytests/pkg/integration/test_python.py | 3 -- .../pytests/pkg/integration/test_salt_user.py | 6 ---- tests/pytests/pkg/integration/test_version.py | 7 ----- .../pytests/pkg/upgrade/test_salt_upgrade.py | 14 ++++------ 8 files changed, 8 insertions(+), 66 deletions(-) diff --git a/tests/pytests/pkg/conftest.py b/tests/pytests/pkg/conftest.py index b5596bb4371e..e29a5a8b8c45 100644 --- a/tests/pytests/pkg/conftest.py +++ b/tests/pytests/pkg/conftest.py @@ -350,18 +350,7 @@ def salt_master(salt_factories, install_salt, pkg_tests_account): master_script = False if platform.is_windows(): - if install_salt.classic: - master_script = True - if install_salt.relenv: - master_script = True - elif not install_salt.upgrade: - master_script = True - if ( - not install_salt.relenv - and install_salt.use_prev_version - and not install_salt.classic - ): - master_script = False + master_script = True if master_script: salt_factories.system_service = False @@ -370,10 +359,7 @@ def salt_master(salt_factories, install_salt, pkg_tests_account): scripts_dir.mkdir(exist_ok=True) salt_factories.scripts_dir = scripts_dir python_executable = install_salt.bin_dir / "Scripts" / "python.exe" - if install_salt.classic: - python_executable = install_salt.bin_dir / "python.exe" - if install_salt.relenv: - python_executable = install_salt.install_dir / "Scripts" / "python.exe" + python_executable = install_salt.install_dir / "Scripts" / "python.exe" salt_factories.python_executable = python_executable factory = salt_factories.salt_master_daemon( random_string("master-"), @@ -384,10 +370,6 @@ def salt_master(salt_factories, install_salt, pkg_tests_account): ) salt_factories.system_service = True else: - - if install_salt.classic and platform.is_darwin(): - os.environ["PATH"] += ":/opt/salt/bin" - factory = salt_factories.salt_master_daemon( random_string("master-"), defaults=config_defaults, @@ -458,12 +440,6 @@ def salt_minion(salt_factories, salt_master, install_salt): ) config_overrides["winrepo_source_dir"] = r"salt://win/repo_ng" - if install_salt.classic and platform.is_windows(): - salt_factories.python_executable = None - - if install_salt.classic and platform.is_darwin(): - os.environ["PATH"] += ":/opt/salt/bin" - factory = salt_master.salt_minion_daemon( minion_id, overrides=config_overrides, diff --git a/tests/pytests/pkg/downgrade/test_salt_downgrade.py b/tests/pytests/pkg/downgrade/test_salt_downgrade.py index 251804530b08..a195bb880c72 100644 --- a/tests/pytests/pkg/downgrade/test_salt_downgrade.py +++ b/tests/pytests/pkg/downgrade/test_salt_downgrade.py @@ -98,8 +98,6 @@ def test_salt_downgrade_minion(salt_call_cli, install_salt): bin_file = install_salt.install_dir / "salt-call.bat" else: bin_file = install_salt.install_dir / "salt-call.exe" - elif platform.is_darwin() and install_salt.classic: - bin_file = install_salt.bin_dir / "salt-call" ret = install_salt.proc.run(bin_file, "--version") assert ret.returncode == 0 diff --git a/tests/pytests/pkg/integration/test_clean_zmq_teardown.py b/tests/pytests/pkg/integration/test_clean_zmq_teardown.py index 309493e69aa5..d1dbe325ab24 100644 --- a/tests/pytests/pkg/integration/test_clean_zmq_teardown.py +++ b/tests/pytests/pkg/integration/test_clean_zmq_teardown.py @@ -12,12 +12,6 @@ log = logging.getLogger(__name__) -@pytest.fixture(autouse=True) -def _skip_on_non_relenv(install_salt): - if not install_salt.relenv: - pytest.skip("This test is for relenv versions of salt") - - def test_check_no_import_error(salt_call_cli, salt_master): """ Test that we don't have any errors on teardown of python when using a py-rendered sls file diff --git a/tests/pytests/pkg/integration/test_pip.py b/tests/pytests/pkg/integration/test_pip.py index 849dbbbfb8b6..3dde96ebb7d0 100644 --- a/tests/pytests/pkg/integration/test_pip.py +++ b/tests/pytests/pkg/integration/test_pip.py @@ -74,8 +74,6 @@ def test_pip_install_extras(shell, install_salt, extras_pypath_bin): """ Test salt-pip installs into the correct directory """ - if not install_salt.relenv: - pytest.skip("The extras directory is only in relenv versions") dep = "pep8" extras_keyword = "extras-3" if platform.is_windows(): @@ -125,11 +123,7 @@ def test_pip_non_root( pypath, pkg_tests_account_environ, ): - if install_salt.classic: - pytest.skip("We can install non-root for classic packages") check_path = extras_pypath_bin / "pep8" - if not install_salt.relenv and not install_salt.classic: - check_path = pypath / "pep8" # We should be able to issue a --help without being root ret = subprocess.run( install_salt.binary_paths["salt"] + ["--help"], @@ -179,8 +173,6 @@ def test_pip_install_salt_extension_in_extras(install_salt, extras_pypath, shell Test salt-pip installs into the correct directory and the salt extension is properly loaded. """ - if not install_salt.relenv: - pytest.skip("The extras directory is only in relenv versions") dep = "salt-analytics-framework" dep_version = "0.1.0" diff --git a/tests/pytests/pkg/integration/test_python.py b/tests/pytests/pkg/integration/test_python.py index 9b16cea37964..77d2a82a16c5 100644 --- a/tests/pytests/pkg/integration/test_python.py +++ b/tests/pytests/pkg/integration/test_python.py @@ -6,9 +6,6 @@ @pytest.fixture def python_script_bin(install_salt): - # Tiamat builds run scripts via `salt python` - if not install_salt.relenv and not install_salt.classic: - return install_salt.binary_paths["python"][:1] + ["python"] return install_salt.binary_paths["python"] diff --git a/tests/pytests/pkg/integration/test_salt_user.py b/tests/pytests/pkg/integration/test_salt_user.py index 280b51624e2d..9e3d3f0de004 100644 --- a/tests/pytests/pkg/integration/test_salt_user.py +++ b/tests/pytests/pkg/integration/test_salt_user.py @@ -84,12 +84,6 @@ def pkg_paths_salt_user_exclusions(): return paths -@pytest.fixture(autouse=True) -def _skip_on_non_relenv(install_salt): - if not install_salt.relenv: - pytest.skip("The salt user only exists on relenv versions of salt") - - def test_salt_user_master(salt_master, install_salt): """ Test the correct user is running the Salt Master diff --git a/tests/pytests/pkg/integration/test_version.py b/tests/pytests/pkg/integration/test_version.py index cda029211087..b1bee9d60afa 100644 --- a/tests/pytests/pkg/integration/test_version.py +++ b/tests/pytests/pkg/integration/test_version.py @@ -39,9 +39,6 @@ def test_salt_versions_report_master(install_salt): """ Test running --versions-report on master """ - if not install_salt.relenv and not install_salt.classic: - pytest.skip("Unable to get the python version dynamically from tiamat builds") - test_bin = os.path.join(*install_salt.binary_paths["master"]) python_bin = os.path.join(*install_salt.binary_paths["python"]) ret = install_salt.proc.run(test_bin, "--versions-report") @@ -131,10 +128,6 @@ def test_symlinks_created(version, symlink, install_salt): """ Test symlinks created """ - if install_salt.classic: - pytest.skip("Symlinks not created for classic macos builds, we adjust the path") - if not install_salt.relenv and symlink == "spm": - symlink = "salt-spm" ret = install_salt.proc.run(pathlib.Path("/usr/local/sbin") / symlink, "--version") ret.stdout.matcher.fnmatch_lines([f"*{version}*"]) diff --git a/tests/pytests/pkg/upgrade/test_salt_upgrade.py b/tests/pytests/pkg/upgrade/test_salt_upgrade.py index 79e7c259fb4e..5bce37d6aebe 100644 --- a/tests/pytests/pkg/upgrade/test_salt_upgrade.py +++ b/tests/pytests/pkg/upgrade/test_salt_upgrade.py @@ -122,8 +122,7 @@ def test_salt_upgrade(salt_call_cli, install_salt): if not install_salt.upgrade: pytest.skip("Not testing an upgrade, do not run") - if install_salt.relenv: - original_py_version = install_salt.package_python_version() + original_py_version = install_salt.package_python_version() # Test pip install before an upgrade dep = "PyGithub==1.56.0" @@ -139,9 +138,8 @@ def test_salt_upgrade(salt_call_cli, install_salt): # pylint: disable=pointless-statement salt_test_upgrade - if install_salt.relenv: - new_py_version = install_salt.package_python_version() - if new_py_version == original_py_version: - # test pip install after an upgrade - use_lib = salt_call_cli.run("--local", "github.get_repo_info", repo) - assert "Authentication information could" in use_lib.stderr + new_py_version = install_salt.package_python_version() + if new_py_version == original_py_version: + # test pip install after an upgrade + use_lib = salt_call_cli.run("--local", "github.get_repo_info", repo) + assert "Authentication information could" in use_lib.stderr From 447e2e44f26b9671da73d9795942ce9aa7c8e800 Mon Sep 17 00:00:00 2001 From: David Murphy Date: Wed, 14 Aug 2024 15:44:01 -0600 Subject: [PATCH 3/4] Limit test to Linux which use systemctl --- tests/pytests/pkg/integration/test_enabled_disabled.py | 2 +- tests/pytests/pkg/integration/test_salt_ufw.py | 5 ++++- tests/pytests/pkg/integration/test_salt_user.py | 3 +-- tests/pytests/pkg/integration/test_systemd_config.py | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/pytests/pkg/integration/test_enabled_disabled.py b/tests/pytests/pkg/integration/test_enabled_disabled.py index 43dd4d4366a0..4cfa5d2adc13 100644 --- a/tests/pytests/pkg/integration/test_enabled_disabled.py +++ b/tests/pytests/pkg/integration/test_enabled_disabled.py @@ -2,7 +2,7 @@ from pytestskipmarkers.utils import platform -@pytest.mark.skip_on_windows(reason="Linux test only") +@pytest.mark.skip_unless_on_linux(reason="Linux test only") def test_services(install_salt, salt_call_cli): """ Check if Services are enabled/disabled diff --git a/tests/pytests/pkg/integration/test_salt_ufw.py b/tests/pytests/pkg/integration/test_salt_ufw.py index eebf0e2f014d..0e0471aebf24 100644 --- a/tests/pytests/pkg/integration/test_salt_ufw.py +++ b/tests/pytests/pkg/integration/test_salt_ufw.py @@ -2,6 +2,10 @@ import pytest +pytestmark = [ + pytest.mark.skip_unless_on_linux, +] + @pytest.fixture def salt_systemd_setup( @@ -26,7 +30,6 @@ def salt_systemd_setup( assert ret.returncode == 0 -@pytest.mark.skip_on_windows @pytest.mark.skip_if_binaries_missing("ufw") def test_salt_ufw(salt_systemd_setup, salt_call_cli, install_salt): """ diff --git a/tests/pytests/pkg/integration/test_salt_user.py b/tests/pytests/pkg/integration/test_salt_user.py index 9e3d3f0de004..fb42ae3c9f6b 100644 --- a/tests/pytests/pkg/integration/test_salt_user.py +++ b/tests/pytests/pkg/integration/test_salt_user.py @@ -9,8 +9,7 @@ from saltfactories.utils.tempfiles import temp_directory pytestmark = [ - pytest.mark.skip_on_windows, - pytest.mark.skip_on_darwin, + pytest.mark.skip_unless_on_linux, ] diff --git a/tests/pytests/pkg/integration/test_systemd_config.py b/tests/pytests/pkg/integration/test_systemd_config.py index 828e4413ad77..5f705eb2ee91 100644 --- a/tests/pytests/pkg/integration/test_systemd_config.py +++ b/tests/pytests/pkg/integration/test_systemd_config.py @@ -3,7 +3,7 @@ import pytest pytestmark = [ - pytest.mark.skip_on_windows(reason="Linux test only"), + pytest.mark.skip_unless_on_linux, ] From 3b755036dbc7a956871eb93170a6937a8882f508 Mon Sep 17 00:00:00 2001 From: David Murphy Date: Thu, 15 Aug 2024 09:44:36 -0600 Subject: [PATCH 4/4] Revised test due to reviewer comments --- tests/pytests/pkg/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/pytests/pkg/conftest.py b/tests/pytests/pkg/conftest.py index e29a5a8b8c45..258d794c89fe 100644 --- a/tests/pytests/pkg/conftest.py +++ b/tests/pytests/pkg/conftest.py @@ -358,7 +358,6 @@ def salt_master(salt_factories, install_salt, pkg_tests_account): scripts_dir = salt_factories.root_dir / "Scripts" scripts_dir.mkdir(exist_ok=True) salt_factories.scripts_dir = scripts_dir - python_executable = install_salt.bin_dir / "Scripts" / "python.exe" python_executable = install_salt.install_dir / "Scripts" / "python.exe" salt_factories.python_executable = python_executable factory = salt_factories.salt_master_daemon(