more fixes related to migrating the mac tests over to functional tests

tests/pytests/functional/modules/test_mac_assistive.py

@@ -33,8 +33,7 @@ def _setup_teardown_vars(assistive, osa_script):
             assistive.remove(osa_script)
 
         smile_bundle = "com.smileonmymac.textexpander"
-        ret = assistive.installed(smile_bundle)
-        smile_bundle_present = ret
+        smile_bundle_present = assistive.installed(smile_bundle)
         if smile_bundle_present:
             assistive.remove(smile_bundle)
 
@@ -56,7 +55,7 @@ def test_installed(assistive, osa_script):
     """
     Tests the True and False return of assistive.installed.
     """
-    # OSA script should have been installed in setUp function
+    # OSA script should have been installed in _setup_teardown_vars function
     ret = assistive.installed(osa_script)
     assert ret
     # Clean up install
@@ -71,7 +70,7 @@ def test_enable(assistive, osa_script):
     """
     Tests setting the enabled status of a bundled ID or command.
     """
-    # OSA script should have been installed and enabled in setUp function
+    # OSA script should have been installed and enabled in _setup_teardown_vars function
     # Now let's disable it, which should return True.
     ret = assistive.enable(osa_script, False)
     assert ret
@@ -91,7 +90,7 @@ def test_enabled(assistive, osa_script):
     """
     Tests if a bundled ID or command is listed in assistive access returns True.
     """
-    # OSA script should have been installed in setUp function, which sets
+    # OSA script should have been installed in _setup_teardown_vars function, which sets
     # enabled to True by default.
     ret = assistive.enabled(osa_script)
     assert ret

tests/pytests/functional/modules/test_mac_brew_pkg.py

@@ -13,15 +13,16 @@ pytestmark = [
     pytest.mark.skip_if_binaries_missing("brew"),
 ]
 
-# Brew doesn't support local package installation - So, let's
-# Grab some small packages available online for brew
-
 
 @pytest.fixture(scope="module")
 def pkg(modules):
     return modules.pkg
 
 
+# Brew doesn't support local package installation - So, let's
+# Grab some small packages available online for brew
+
+
 @pytest.fixture(scope="function")
 def add_pkg():
     yield "algol68g"

tests/pytests/functional/modules/test_mac_desktop.py

@@ -31,16 +31,13 @@ def test_set_output_volume(desktop):
     """
     Tests the return of set_output_volume.
     """
+    current_vol = desktop.get_output_volume()
     try:
-        ret = desktop.get_output_volume()
-        current_vol = ret
         to_set = 10
         if current_vol == str(to_set):
             to_set += 2
-        ret = desktop.set_output_volume(str(to_set))
-        new_vol = ret
-        ret = desktop.get_output_volume()
-        check_vol = ret
+        new_vol = desktop.set_output_volume(str(to_set))
+        check_vol = desktop.get_output_volume()
         assert new_vol == check_vol
     finally:
         # Set volume back to what it was before

tests/pytests/functional/modules/test_mac_power.py

@@ -7,7 +7,6 @@ import pytest
 from salt.exceptions import SaltInvocationError
 
 pytestmark = [
-    pytest.mark.flaky(max_runs=10),
     pytest.mark.skip_if_binaries_missing("systemsetup"),
     pytest.mark.slow_test,
     pytest.mark.destructive_test,

tests/pytests/functional/modules/test_mac_service.py

@@ -147,13 +147,14 @@ def test_stop(service, service_name):
         ret = service.stop("spongebob")
         assert "Service not found" in str(exc.value)
 
+    service.start(service_name)
+
 
 def test_status(service, service_name):
     """
     Test service.status
     """
     # A running service
-    service.start(service_name)
     ret = service.status(service_name)
     assert ret
 
@@ -166,6 +167,8 @@ def test_status(service, service_name):
     ret = service.status("spongebob")
     assert not ret
 
+    service.start(service_name)
+
 
 def test_available(service, service_name):
     """
@@ -206,9 +209,6 @@ def test_disabled(service, service_name):
     """
     Test service.disabled
     """
-    service.enabled(service_name)
-    service.start(service_name)
-
     ret = service.disabled(service_name)
     assert not ret
 

tests/pytests/functional/modules/test_mac_shadow.py

@@ -8,9 +8,10 @@ import types
 import pytest
 from saltfactories.utils import random_string
 
+from salt.exceptions import CommandExecutionError
+
 pytestmark = [
     pytest.mark.skip_if_binaries_missing("dscl", "pwpolicy"),
-    pytest.mark.skip_initial_gh_actions_failure,
     pytest.mark.slow_test,
     pytest.mark.skip_if_not_root,
     pytest.mark.skip_unless_on_darwin,
@@ -54,10 +55,9 @@ def test_get_account_created(shadow, accounts):
     assert isinstance(obj_date, datetime.date)
 
     # User does not exist
-    assert (
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.get_account_created(accounts.not_created)
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
 
 
 def test_get_last_change(shadow, accounts):
@@ -71,10 +71,9 @@ def test_get_last_change(shadow, accounts):
     assert isinstance(obj_date, datetime.date)
 
     # User does not exist
-    assert (
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.get_last_change(accounts.not_created)
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
 
 
 def test_get_login_failed_last(shadow, accounts):
@@ -88,10 +87,9 @@ def test_get_login_failed_last(shadow, accounts):
     assert isinstance(obj_date, datetime.date)
 
     # User does not exist
-    assert (
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.get_login_failed_last(accounts)
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
 
 
 def test_get_login_failed_count(shadow, accounts):
@@ -102,10 +100,9 @@ def test_get_login_failed_count(shadow, accounts):
     assert shadow.get_login_failed_count(accounts.created) == "0"
 
     # User does not exist
-    assert (
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.get_login_failed_count(accounts.not_created)
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
 
 
 def test_get_set_maxdays(shadow, accounts):
@@ -118,14 +115,13 @@ def test_get_set_maxdays(shadow, accounts):
     assert shadow.get_maxdays(accounts.created) == 20
 
     # User does not exist
-    assert (
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.set_maxdays(accounts.not_created, 7)
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
-    assert (
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
+
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.get_maxdays(accounts.not_created)
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
 
 
 def test_get_set_change(shadow, accounts):
@@ -138,14 +134,13 @@ def test_get_set_change(shadow, accounts):
     assert shadow.get_change(accounts.created) == "02/11/2011"
 
     # User does not exist
-    assert (
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.set_change(accounts.not_created, "02/11/2012")
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
-    assert (
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
+
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.get_change(accounts.not_created)
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
 
 
 def test_get_set_expire(shadow, accounts):
@@ -158,14 +153,13 @@ def test_get_set_expire(shadow, accounts):
     assert shadow.get_expire(accounts.created) == "02/11/2011"
 
     # User does not exist
-    assert (
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.set_expire(accounts.not_created, "02/11/2012")
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
-    assert (
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
+
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.get_expire(accounts.not_created)
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
 
 
 def test_del_password(shadow, accounts):
@@ -177,10 +171,9 @@ def test_del_password(shadow, accounts):
     assert shadow.info(accounts.created)["passwd"] == "*"
 
     # User does not exist
-    assert (
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.del_password(accounts.not_created)
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)
 
 
 def test_set_password(shadow, accounts):
@@ -191,7 +184,6 @@ def test_set_password(shadow, accounts):
     assert shadow.set_password(accounts.created, "Pa$$W0rd")
 
     # User does not exist
-    assert (
+    with pytest.raises(CommandExecutionError) as exc:
         shadow.set_password(accounts.not_created, "P@SSw0rd")
-        == f"ERROR: User not found: {accounts.not_created}"
-    )
+        assert f"ERROR: User not found: {accounts.not_created}" in str(exc.value)

tests/pytests/functional/modules/test_mac_softwareupdate.py

@@ -4,12 +4,13 @@ integration tests for mac_softwareupdate
 
 import pytest
 
+from salt.exceptions import SaltInvocationError
+
 pytestmark = [
     pytest.mark.slow_test,
     pytest.mark.skip_if_binaries_missing("softwareupdate"),
     pytest.mark.skip_if_not_root,
     pytest.mark.skip_unless_on_darwin,
-    pytest.mark.skip_initial_gh_actions_failure,
 ]
 
 
@@ -82,6 +83,7 @@ def test_ignore(softwareupdate):
     assert "squidward" in ret
 
 
+@pytest.mark.skip(reason="Ignore schedule support removed from latest OS X.")
 def test_schedule(softwareupdate):
     """
     Test softwareupdate.schedule_enable
@@ -121,8 +123,9 @@ def test_update(softwareupdate):
     assert not ret
 
     # Test update not available
-    ret = softwareupdate.update("spongebob")
-    assert "Update not available" in ret
+    with pytest.raises(SaltInvocationError) as exc:
+        ret = softwareupdate.update("spongebob")
+        assert "Update not available" in str(exc.value)
 
 
 def test_list_downloads(softwareupdate):
@@ -141,8 +144,9 @@ def test_download(softwareupdate):
     the download function
     """
     # Test update not available
-    ret = softwareupdate.download("spongebob")
-    assert "Update not available" in ret
+    with pytest.raises(SaltInvocationError) as exc:
+        softwareupdate.download("spongebob")
+        assert "Update not available" in str(exc.value)
 
 
 def test_download_all(softwareupdate):
@@ -153,6 +157,7 @@ def test_download_all(softwareupdate):
     assert isinstance(ret, list)
 
 
+@pytest.mark.skip(reason="Ignore catalog support removed from latest OS X.")
 def test_get_set_reset_catalog(softwareupdate):
     """
     Test softwareupdate.download_all

tests/pytests/functional/modules/test_mac_timezone.py

@@ -14,7 +14,6 @@ import datetime
 import pytest
 
 pytestmark = [
-    pytest.mark.flaky(max_runs=4),
     pytest.mark.skip_if_binaries_missing("systemsetup"),
     pytest.mark.slow_test,
     pytest.mark.destructive_test,

tests/pytests/functional/modules/test_mac_xattr.py

@@ -4,6 +4,8 @@ integration tests for mac_xattr
 
 import pytest
 
+from salt.exceptions import CommandExecutionError
+
 pytestmark = [
     pytest.mark.skip_if_binaries_missing("xattr"),
     pytest.mark.slow_test,
@@ -13,8 +15,8 @@ pytestmark = [
 
 
 @pytest.fixture(scope="module")
-def xttr(modules):
-    return modules.xttr
+def xattr(modules):
+    return modules.xattr
 
 
 @pytest.fixture(scope="function")
@@ -47,8 +49,9 @@ def test_list_no_xattr(xattr, setup_teardown_vars):
     assert ret == {}
 
     # Test file not found
-    ret = xattr.list(no_file)
-    assert f"File not found: {no_file}" in ret
+    with pytest.raises(CommandExecutionError) as exc:
+        ret = xattr.list(no_file)
+        assert f"File not found: {no_file}" in str(exc.value)
 
 
 def test_write(xattr, setup_teardown_vars):
@@ -81,8 +84,9 @@ def test_write(xattr, setup_teardown_vars):
     }
 
     # Test file not found
-    ret = xattr.write(no_file, "patrick", "jellyfish")
-    assert f"File not found: {no_file}" in ret
+    with pytest.raises(CommandExecutionError) as exc:
+        ret = xattr.write(no_file, "patrick", "jellyfish")
+        assert f"File not found: {no_file}" in str(exc.value)
 
 
 def test_read(xattr, setup_teardown_vars):
@@ -105,12 +109,14 @@ def test_read(xattr, setup_teardown_vars):
     assert ret == "squarepants"
 
     # Test file not found
-    ret = xattr.read(no_file, "spongebob")
-    assert f"File not found: {no_file}" in ret
+    with pytest.raises(CommandExecutionError) as exc:
+        ret = xattr.read(no_file, "spongebob")
+        assert f"File not found: {no_file}" in str(exc.value)
 
     # Test attribute not found
-    ret = xattr.read(test_file, "patrick")
-    assert "Attribute not found: patrick" in ret
+    with pytest.raises(CommandExecutionError) as exc:
+        ret = xattr.read(test_file, "patrick")
+        assert "Attribute not found: patrick" in str(exc.value)
 
 
 def test_delete(xattr, setup_teardown_vars):
@@ -146,12 +152,14 @@ def test_delete(xattr, setup_teardown_vars):
     }
 
     # Test file not found
-    ret = xattr.delete(no_file, "spongebob")
-    assert f"File not found: {no_file}" in ret
+    with pytest.raises(CommandExecutionError) as exc:
+        ret = xattr.delete(no_file, "spongebob")
+        assert f"File not found: {no_file}" in str(exc.value)
 
     # Test attribute not found
-    ret = xattr.delete(test_file, "patrick")
-    assert "Attribute not found: patrick" in ret
+    with pytest.raises(CommandExecutionError) as exc:
+        ret = xattr.delete(test_file, "patrick")
+        assert "Attribute not found: patrick" in str(exc.value)
 
 
 def test_clear(xattr, setup_teardown_vars):
@@ -180,5 +188,6 @@ def test_clear(xattr, setup_teardown_vars):
     assert ret
 
     # Test file not found
-    ret = xattr.clear(no_file)
-    assert f"File not found: {no_file}" in ret
+    with pytest.raises(CommandExecutionError) as exc:
+        ret = xattr.clear(no_file)
+        assert f"File not found: {no_file}" in str(exc.value)