Some more test timeout updates

Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>

tests/integration/modules/test_gem.py

@@ -18,6 +18,7 @@ def check_status():
         return False
 
 
+@pytest.mark.timeout_unless_on_windows(120)
 @pytest.mark.skip_if_binaries_missing("gem")
 @pytest.mark.windows_whitelisted
 @pytest.mark.destructive_test

tests/integration/modules/test_localemod.py

@@ -27,6 +27,7 @@ class LocaleModuleTest(ModuleCase):
         locale = self.run_function("locale.get_locale")
         self.assertNotIn("Unsupported platform!", locale)
 
+    @pytest.mark.timeout(120)
     @pytest.mark.destructive_test
     @pytest.mark.slow_test
     def test_gen_locale(self):

tests/integration/ssh/test_state.py

@@ -55,6 +55,7 @@ class SSHStateTest(SSHCase):
         self.assertTrue(check_file)
 
     @pytest.mark.slow_test
+    @pytest.mark.timeout_unless_on_windows(120)
     def test_state_sls_id(self):
         """
         test state.sls_id with salt-ssh

tests/pytests/functional/channel/test_server.py

@@ -28,7 +28,8 @@ log = logging.getLogger(__name__)
 pytestmark = [
     pytest.mark.skip_on_spawning_platform(
         reason="These tests are currently broken on spawning platforms. Need to be rewritten.",
-    )
+    ),
+    pytest.mark.timeout_unless_on_windows(120),
 ]
 
 

tests/pytests/functional/modules/test_mac_brew_pkg.py

@@ -7,6 +7,7 @@ import pytest
 
 pytestmark = [
     pytest.mark.slow_test,
+    pytest.mark.timeout(120),
     pytest.mark.destructive_test,
     pytest.mark.skip_if_not_root,
     pytest.mark.skip_unless_on_darwin,
@@ -49,7 +50,6 @@ def pkg_2_name(pkg):
             pkg.remove(pkg_name)
 
 
-@pytest.mark.timeout(120)
 def test_brew_install(pkg, pkg_1_name):
     """
     Tests the installation of packages

tests/pytests/functional/modules/test_mac_softwareupdate.py

@@ -8,6 +8,7 @@ from salt.exceptions import SaltInvocationError
 
 pytestmark = [
     pytest.mark.slow_test,
+    pytest.mark.timeout(240),
     pytest.mark.skip_if_binaries_missing("softwareupdate"),
     pytest.mark.skip_if_not_root,
     pytest.mark.skip_unless_on_darwin,

tests/pytests/functional/modules/test_pkg.py

@@ -3,7 +3,6 @@ import logging
 import os
 import re
 import shutil
-import tempfile
 import time
 
 import pytest
@@ -15,6 +14,10 @@ import salt.utils.platform
 
 log = logging.getLogger(__name__)
 
+pytestmark = [
+    pytest.mark.timeout_unless_on_windows(240),
+]
+
 
 @pytest.fixture
 def ctx():
@@ -22,25 +25,24 @@ def ctx():
 
 
 @pytest.fixture
-def preserve_rhel_yum_conf():
+def _preserve_rhel_yum_conf(tmp_path):
 
     # save off current yum.conf
     cfg_file = "/etc/yum.conf"
     if not os.path.exists(cfg_file):
         pytest.skip("Only runs on RedHat.")
 
-    tmp_dir = str(tempfile.gettempdir())
-    tmp_file = os.path.join(tmp_dir, "yum.conf")
+    tmp_file = tmp_path / "yum.conf"
     shutil.copy2(cfg_file, tmp_file)
-    yield
-
-    # restore saved yum.conf
-    shutil.copy2(tmp_file, cfg_file)
-    os.remove(tmp_file)
+    try:
+        yield
+    finally:
+        # restore saved yum.conf
+        shutil.copy2(tmp_file, cfg_file)
 
 
 @pytest.fixture
-def refresh_db(ctx, grains, modules):
+def _refresh_db(ctx, grains, modules):
     if "refresh" not in ctx:
         modules.pkg.refresh_db()
         ctx["refresh"] = True
@@ -73,9 +75,10 @@ def test_pkg(grains):
     return _pkg
 
 
+@pytest.mark.usefixtures("_refresh_db")
 @pytest.mark.requires_salt_modules("pkg.list_pkgs")
 @pytest.mark.slow_test
-def test_list(modules, refresh_db):
+def test_list(modules):
     """
     verify that packages are installed
     """
@@ -107,11 +110,12 @@ def test_version_cmp(grains, modules):
     assert modules.pkg.version_cmp(*gt) == 1
 
 
+@pytest.mark.usefixtures("_refresh_db")
 @pytest.mark.destructive_test
 @pytest.mark.requires_salt_modules("pkg.mod_repo", "pkg.del_repo", "pkg.get_repo")
 @pytest.mark.slow_test
 @pytest.mark.requires_network
-def test_mod_del_repo(grains, modules, refresh_db):
+def test_mod_del_repo(grains, modules):
     """
     test modifying and deleting a software repository
     """
@@ -161,7 +165,8 @@ def test_mod_del_repo(grains, modules, refresh_db):
 
 
 @pytest.mark.slow_test
-def test_mod_del_repo_multiline_values(modules, refresh_db):
+@pytest.mark.usefixtures("_refresh_db")
+def test_mod_del_repo_multiline_values(modules):
     """
     test modifying and deleting a software repository defined with multiline values
     """
@@ -175,7 +180,6 @@ def test_mod_del_repo_multiline_values(modules, refresh_db):
             expected_get_repo_baseurl = (
                 "http://my.fake.repo/foo/bar/\nhttp://my.fake.repo.alt/foo/bar/"
             )
-            major_release = int(modules.grains.item("osmajorrelease")["osmajorrelease"])
             repo = "fakerepo"
             name = "Fake repo for RHEL/CentOS/SUSE"
             baseurl = my_baseurl
@@ -224,16 +228,16 @@ def test_which(modules):
     """
     test finding the package owning a file
     """
-    func = "pkg.which"
     ret = modules.pkg.which("/usr/local/bin/salt-call")
     assert len(ret) != 0
 
 
+@pytest.mark.usefixtures("_refresh_db")
 @pytest.mark.destructive_test
 @pytest.mark.requires_salt_modules("pkg.version", "pkg.install", "pkg.remove")
 @pytest.mark.slow_test
 @pytest.mark.requires_network
-def test_install_remove(modules, test_pkg, refresh_db):
+def test_install_remove(modules, test_pkg):
     """
     successfully install and uninstall a package
     """
@@ -258,6 +262,7 @@ def test_install_remove(modules, test_pkg, refresh_db):
         test_remove()
 
 
+@pytest.mark.usefixtures("_refresh_db")
 @pytest.mark.destructive_test
 @pytest.mark.skip_on_photonos(
     reason="package hold/unhold unsupported on Photon OS",
@@ -273,7 +278,7 @@ def test_install_remove(modules, test_pkg, refresh_db):
 @pytest.mark.slow_test
 @pytest.mark.requires_network
 @pytest.mark.requires_salt_states("pkg.installed")
-def test_hold_unhold(grains, modules, states, test_pkg, refresh_db):
+def test_hold_unhold(grains, modules, states, test_pkg):
     """
     test holding and unholding a package
     """
@@ -315,11 +320,12 @@ def test_hold_unhold(grains, modules, states, test_pkg, refresh_db):
             assert ret.result is True
 
 
+@pytest.mark.usefixtures("_refresh_db")
 @pytest.mark.destructive_test
 @pytest.mark.requires_salt_modules("pkg.refresh_db")
 @pytest.mark.slow_test
 @pytest.mark.requires_network
-def test_refresh_db(grains, tmp_path, minion_opts, refresh_db):
+def test_refresh_db(grains, minion_opts):
     """
     test refreshing the package database
     """
@@ -344,9 +350,10 @@ def test_refresh_db(grains, tmp_path, minion_opts, refresh_db):
     assert os.path.isfile(rtag) is False
 
 
+@pytest.mark.usefixtures("_refresh_db")
 @pytest.mark.requires_salt_modules("pkg.info_installed")
 @pytest.mark.slow_test
-def test_pkg_info(grains, modules, test_pkg, refresh_db):
+def test_pkg_info(grains, modules, test_pkg):
     """
     Test returning useful information on Ubuntu systems.
     """
@@ -371,6 +378,7 @@ def test_pkg_info(grains, modules, test_pkg, refresh_db):
         assert test_pkg in keys
 
 
+@pytest.mark.usefixtures("_refresh_db")
 @pytest.mark.skipif(True, reason="Temporary Skip - Causes centos 8 test to fail")
 @pytest.mark.destructive_test
 @pytest.mark.requires_salt_modules(
@@ -382,7 +390,7 @@ def test_pkg_info(grains, modules, test_pkg, refresh_db):
 )
 @pytest.mark.slow_test
 @pytest.mark.requires_network
-def test_pkg_upgrade_has_pending_upgrades(grains, modules, test_pkg, refresh_db):
+def test_pkg_upgrade_has_pending_upgrades(grains, modules):
     """
     Test running a system upgrade when there are packages that need upgrading
     """
@@ -452,6 +460,7 @@ def test_pkg_upgrade_has_pending_upgrades(grains, modules, test_pkg, refresh_db)
             assert ret != {}
 
 
+@pytest.mark.usefixtures("_refresh_db")
 @pytest.mark.destructive_test
 @pytest.mark.skip_on_darwin(
     reason="The jenkins user is equivalent to root on mac, causing the test to be unrunnable"
@@ -459,7 +468,7 @@ def test_pkg_upgrade_has_pending_upgrades(grains, modules, test_pkg, refresh_db)
 @pytest.mark.requires_salt_modules("pkg.remove", "pkg.latest_version")
 @pytest.mark.slow_test
 @pytest.mark.requires_salt_states("pkg.removed")
-def test_pkg_latest_version(grains, modules, states, test_pkg, refresh_db):
+def test_pkg_latest_version(grains, modules, states, test_pkg):
     """
     Check that pkg.latest_version returns the latest version of the uninstalled package.
     The package is not installed. Only the package version is checked.
@@ -497,10 +506,11 @@ def test_pkg_latest_version(grains, modules, states, test_pkg, refresh_db):
     assert pkg_latest in cmd_pkg
 
 
+@pytest.mark.usefixtures("_preserve_rhel_yum_conf")
 @pytest.mark.destructive_test
 @pytest.mark.requires_salt_modules("pkg.list_repos")
 @pytest.mark.slow_test
-def test_list_repos_duplicate_entries(preserve_rhel_yum_conf, grains, modules):
+def test_list_repos_duplicate_entries(grains, modules):
     """
     test duplicate entries in /etc/yum.conf
 
@@ -531,13 +541,13 @@ def test_list_repos_duplicate_entries(preserve_rhel_yum_conf, grains, modules):
     # test explicitly strict_config
     expected = "While reading from '/etc/yum.conf' [line  8]: option 'http_caching' in section 'main' already exists"
     with pytest.raises(configparser.DuplicateOptionError) as exc_info:
-        result = modules.pkg.list_repos(strict_config=True)
-    assert "{}".format(exc_info.value) == expected
+        modules.pkg.list_repos(strict_config=True)
+    assert str(exc_info.value) == expected
 
     # test implicitly strict_config
     with pytest.raises(configparser.DuplicateOptionError) as exc_info:
-        result = modules.pkg.list_repos()
-    assert "{}".format(exc_info.value) == expected
+        modules.pkg.list_repos()
+    assert str(exc_info.value) == expected
 
 
 @pytest.mark.destructive_test

tests/pytests/functional/states/pkgrepo/test_debian.py

@@ -6,7 +6,6 @@ import shutil
 import sys
 from sysconfig import get_path
 
-import _pytest._version
 import attr
 import pytest
 
@@ -15,12 +14,10 @@ import salt.utils.files
 from tests.conftest import CODE_DIR
 from tests.support.mock import MagicMock, patch
 
-PYTEST_GE_7 = getattr(_pytest._version, "version_tuple", (-1, -1)) >= (7, 0)
-
-
 log = logging.getLogger(__name__)
 
 pytestmark = [
+    pytest.mark.timeout_unless_on_windows(120),
     pytest.mark.destructive_test,
     pytest.mark.skip_if_not_root,
     pytest.mark.slow_test,
@@ -30,11 +27,8 @@ pytestmark = [
 @pytest.fixture
 def pkgrepo(states, grains):
     if grains["os_family"] != "Debian":
-        exc_kwargs = {}
-        if PYTEST_GE_7:
-            exc_kwargs["_use_item_location"] = True
         raise pytest.skip.Exception(
-            "Test only for debian based platforms", **exc_kwargs
+            "Test only for debian based platforms", _use_item_location=True
         )
     return states.pkgrepo
 
@@ -102,12 +96,9 @@ def system_aptsources_ids(value):
 def system_aptsources(request, grains):
     sys_modules = list(sys.modules)
     copied_paths = []
-    exc_kwargs = {}
-    if PYTEST_GE_7:
-        exc_kwargs["_use_item_location"] = True
     if grains["os_family"] != "Debian":
         raise pytest.skip.Exception(
-            "Test only for debian based platforms", **exc_kwargs
+            "Test only for debian based platforms", _use_item_location=True
         )
     try:
         try:
@@ -117,7 +108,7 @@ def system_aptsources(request, grains):
                 raise pytest.skip.Exception(
                     "This test is meant to run without the system aptsources package, but it's "
                     "available from '{}'.".format(sourceslist.__file__),
-                    **exc_kwargs,
+                    _use_item_location=True,
                 )
             else:
                 # Run the test
@@ -162,7 +153,8 @@ def system_aptsources(request, grains):
                             shutil.copyfile(src, dst)
                 if not copied_paths:
                     raise pytest.skip.Exception(
-                        "aptsources.sourceslist python module not found", **exc_kwargs
+                        "aptsources.sourceslist python module not found",
+                        _use_item_location=True,
                     )
                 # Run the test
                 yield request.param

tests/pytests/functional/test_version.py

@@ -9,6 +9,7 @@ from tests.support.pytest.helpers import FakeSaltExtension
 pytestmark = [
     # These are slow because they create a virtualenv and install salt in it
     pytest.mark.slow_test,
+    pytest.mark.timeout(120),
 ]
 
 log = logging.getLogger(__name__)

tests/pytests/integration/minion/test_job_return.py

@@ -91,9 +91,10 @@ def salt_minion_1(salt_master_1, salt_master_2):
         yield factory
 
 
-def test_job_resturn(salt_master_1, salt_master_2, salt_minion_1):
+@pytest.mark.timeout_unless_on_windows(360)
+def test_job_return(salt_master_1, salt_master_2, salt_minion_1):
     cli = salt_master_1.salt_cli(timeout=120)
-    ret = cli.run("test.ping", "-v", minion_tgt="minion-1")
+    ret = cli.run("test.ping", "-v", minion_tgt=salt_minion_1.id)
     for line in ret.stdout.splitlines():
         if "with jid" in line:
             jid = line.split("with jid")[1].strip()

tests/pytests/integration/minion/test_return_retries.py

@@ -44,17 +44,19 @@ def test_publish_retry(salt_master, salt_minion_retry, salt_cli, salt_run_cli):
         time.sleep(5)
 
     data = None
-    for i in range(1, 30):
+    for _ in range(1, 30):
         time.sleep(1)
         data = salt_run_cli.run("jobs.lookup_jid", jid, _timeout=60).data
         if data:
             break
 
+    assert data
     assert salt_minion_retry.id in data
     assert data[salt_minion_retry.id] is True
 
 
 @pytest.mark.slow_test
+@pytest.mark.timeout_unless_on_windows(180)
 def test_pillar_timeout(salt_master_factory, tmp_path):
     cmd = 'print(\'{"foo": "bar"}\');\n'
 

tests/pytests/integration/modules/grains/test_append.py

@@ -106,7 +106,7 @@ def test_grains_append_val_is_list(salt_call_cli, append_grain):
     assert ret.data == {append_grain.key: [append_grain.value, second_grain]}
 
 
-@pytest.mark.timeout_unless_on_windows(120)
+@pytest.mark.timeout_unless_on_windows(240)
 def test_grains_remove_add(
     salt_call_cli, append_grain, wait_for_pillar_refresh_complete
 ):

tests/pytests/integration/modules/test_virt.py

@@ -15,6 +15,7 @@ log = logging.getLogger(__name__)
 
 pytestmark = [
     pytest.mark.slow_test,
+    pytest.mark.timeout_unless_on_windows(120, func_only=True),
     pytest.mark.skip_if_binaries_missing("docker"),
 ]
 

tests/pytests/integration/netapi/test_ssh_client.py

@@ -135,16 +135,19 @@ def test_ssh_disabled(client, auth_creds):
     assert ret is None
 
 
-@pytest.mark.timeout_unless_on_windows(120)
+@pytest.mark.timeout_unless_on_windows(360, func_only=True)
 def test_shell_inject_ssh_priv(
-    client, salt_ssh_roster_file, rosters_dir, tmp_path, salt_auto_account
+    client, salt_ssh_roster_file, rosters_dir, tmp_path, salt_auto_account, grains
 ):
     """
     Verify CVE-2020-16846 for ssh_priv variable
     """
+    if grains["os"] == "VMware Photon OS" and grains["osmajorrelease"] == 3:
+        pytest.skip("Skipping problematic test on PhotonOS 3")
     # ZDI-CAN-11143
     path = tmp_path / "test-11143"
     tgts = ["repo.saltproject.io", "www.zerodayinitiative.com"]
+    ret = None
     for tgt in tgts:
         low = {
             "roster": "cache",
@@ -161,7 +164,9 @@ def test_shell_inject_ssh_priv(
         ret = client.run(low)
         if ret:
             break
+
     assert path.exists() is False
+    assert ret
     assert not ret[tgt]["stdout"]
     assert ret[tgt]["stderr"]
 

tests/pytests/integration/runners/state/orchestrate/test_orchestrate.py

@@ -489,6 +489,7 @@ def _check_skip(grains):
     return False
 
 
+@pytest.mark.timeout_unless_on_windows(120)
 @pytest.mark.skip_initial_gh_actions_failure(skip=_check_skip)
 def test_orchestrate_subset(
     salt_run_cli,

tests/pytests/integration/states/test_ansiblegate.py

@@ -65,6 +65,7 @@ def ansible_inventory(ansible_inventory_directory, sshd_server):
 
 
 @pytest.mark.requires_sshd_server
+@pytest.mark.timeout_unless_on_windows(120)
 def test_ansible_playbook(salt_call_cli, ansible_inventory, tmp_path):
     rundir = tmp_path / "rundir"
     rundir.mkdir(exist_ok=True, parents=True)

tests/pytests/integration/states/test_x509_v2.py

@@ -33,6 +33,7 @@ log = logging.getLogger(__name__)
 
 pytestmark = [
     pytest.mark.slow_test,
+    pytest.mark.timeout_unless_on_windows(120),
     pytest.mark.skipif(HAS_LIBS is False, reason="Needs cryptography library"),
 ]
 

tests/pytests/unit/grains/test_core.py

@@ -2326,6 +2326,7 @@ def test_fqdns_return():
 
 
 @pytest.mark.skip_unless_on_linux
+@pytest.mark.timeout(60)
 def test_fqdns_socket_error(caplog):
     """
     test the behavior on non-critical socket errors of the dns grain