Reduce duplication, de-clutter, simplify

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

tests/pytests/pkg/conftest.py

@@ -12,15 +12,8 @@ from saltfactories.utils import random_string
 from saltfactories.utils.tempfiles import SaltPillarTree, SaltStateTree
 
 import salt.config
-from tests.pytests.pkg.support.helpers import (
-    CODE_DIR,
-    TESTS_DIR,
-    ApiRequest,
-    SaltMaster,
-    SaltMasterWindows,
-    SaltPkgInstall,
-    TestUser,
-)
+from tests.conftest import CODE_DIR, TESTS_DIR
+from tests.support.pkg import ApiRequest, SaltMaster, SaltMasterWindows, SaltPkgInstall
 
 log = logging.getLogger(__name__)
 
@@ -303,7 +296,9 @@ def sls(state_tree):
 
 
 @pytest.fixture(scope="session")
-def salt_master(salt_factories, install_salt, state_tree, pillar_tree):
+def salt_master(
+    salt_factories, install_salt, state_tree, pillar_tree, pkg_tests_account
+):
     """
     Start up a master
     """
@@ -327,7 +322,13 @@ def salt_master(salt_factories, install_salt, state_tree, pillar_tree):
         "pillar_roots": pillar_tree.as_dict(),
         "rest_cherrypy": {"port": 8000, "disable_ssl": True},
         "netapi_enable_clients": ["local"],
-        "external_auth": {"auto": {"saltdev": [".*"]}},
+        "external_auth": {
+            "auto": {
+                pkg_tests_account.username: [
+                    ".*",
+                ],
+            },
+        },
         "fips_mode": FIPS_TESTRUN,
         "open_mode": True,
     }
@@ -520,9 +521,9 @@ def salt_call_cli(salt_minion):
     return salt_minion.salt_call_cli()
 
 
-@pytest.fixture(scope="module")
-def test_account(salt_call_cli):
-    with TestUser(salt_call_cli=salt_call_cli) as account:
+@pytest.fixture(scope="session")
+def pkg_tests_account():
+    with pytest.helpers.create_account() as account:
         yield account
 
 
@@ -557,6 +558,8 @@ def salt_api(salt_master, install_salt, extras_pypath):
 
 
 @pytest.fixture(scope="module")
-def api_request(test_account, salt_api):
-    with ApiRequest(salt_api=salt_api, test_account=test_account) as session:
+def api_request(pkg_tests_account, salt_api):
+    with ApiRequest(
+        port=salt_api.config["rest_cherrypy"]["port"], account=pkg_tests_account
+    ) as session:
         yield session

tests/pytests/pkg/integration/test_check_imports.py

@@ -10,11 +10,77 @@ pytestmark = [
 log = logging.getLogger(__name__)
 
 
-def test_check_imports(salt_cli, salt_minion):
+CHECK_IMPORTS_SLS_CONTENTS = """
+#!py
+import importlib
+
+def run():
+    config = {}
+    for test_import in [
+        'templates', 'platform', 'cli', 'executors', 'config', 'wheel', 'netapi',
+        'cache', 'proxy', 'transport', 'metaproxy', 'modules', 'tokens', 'matchers',
+        'acl', 'auth', 'log', 'engines', 'client', 'returners', 'runners', 'tops',
+        'output', 'daemons', 'thorium', 'renderers', 'states', 'cloud', 'roster',
+        'beacons', 'pillar', 'spm', 'utils', 'sdb', 'fileserver', 'defaults',
+        'ext', 'queues', 'grains', 'serializers'
+    ]:
+        try:
+            import_name = "salt.{}".format(test_import)
+            importlib.import_module(import_name)
+            config['test_imports_succeeded'] = {
+                'test.succeed_without_changes': [
+                    {
+                        'name': import_name
+                    },
+                ],
+            }
+        except ModuleNotFoundError as err:
+            config['test_imports_failed'] = {
+                'test.fail_without_changes': [
+                    {
+                        'name': import_name,
+                        'comment': "The imports test failed. The error was: {}".format(err)
+                    },
+                ],
+            }
+
+    for stdlib_import in ["telnetlib"]:
+        try:
+            importlib.import_module(stdlib_import)
+            config['stdlib_imports_succeeded'] = {
+                'test.succeed_without_changes': [
+                    {
+                        'name': stdlib_import
+                    },
+                ],
+            }
+        except ModuleNotFoundError as err:
+            config['stdlib_imports_failed'] = {
+                'test.fail_without_changes': [
+                    {
+                        'name': stdlib_import,
+                        'comment': "The stdlib imports test failed. The error was: {}".format(err)
+                    },
+                ],
+            }
+    return config
+"""
+
+
+@pytest.fixture
+def state_name(salt_master):
+    name = "check-imports"
+    with salt_master.state_tree.base.temp_file(
+        f"{name}.sls", CHECK_IMPORTS_SLS_CONTENTS
+    ):
+        yield name
+
+
+def test_check_imports(salt_cli, salt_minion, state_name):
     """
     Test imports
     """
-    ret = salt_cli.run("state.sls", "check_imports", minion_tgt=salt_minion.id)
+    ret = salt_cli.run("state.sls", state_name, minion_tgt=salt_minion.id)
     assert ret.returncode == 0
     assert ret.data
     result = MultiStateResult(raw=ret.data)

tests/pytests/pkg/integration/test_pip.py

@@ -42,6 +42,14 @@ def wipe_pydeps(shell, install_salt, extras_pypath):
             shutil.rmtree(dirname, ignore_errors=True)
 
 
+@pytest.fixture
+def pkg_tests_account_environ(pkg_tests_account):
+    environ = os.environ.copy()
+    environ["LOGNAME"] = environ["USER"] = pkg_tests_account.username
+    environ["HOME"] = pkg_tests_account.info.home
+    return environ
+
+
 def test_pip_install(salt_call_cli, install_salt, shell):
     """
     Test pip.install and ensure module can use installed library
@@ -98,18 +106,25 @@ def test_pip_install_extras(shell, install_salt, extras_pypath_bin):
     assert ret.returncode == 0
 
 
-def demote(user_uid, user_gid):
+def demote(account):
     def result():
         # os.setgid does not remove group membership, so we remove them here so they are REALLY non-root
         os.setgroups([])
-        os.setgid(user_gid)
-        os.setuid(user_uid)
+        os.setgid(account.info.gid)
+        os.setuid(account.info.uid)
 
     return result
 
 
 @pytest.mark.skip_on_windows(reason="We can't easily demote users on Windows")
-def test_pip_non_root(shell, install_salt, test_account, extras_pypath_bin, pypath):
+def test_pip_non_root(
+    shell,
+    install_salt,
+    pkg_tests_account,
+    extras_pypath_bin,
+    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"
@@ -118,8 +133,8 @@ def test_pip_non_root(shell, install_salt, test_account, extras_pypath_bin, pypa
     # We should be able to issue a --help without being root
     ret = subprocess.run(
         install_salt.binary_paths["salt"] + ["--help"],
-        preexec_fn=demote(test_account.uid, test_account.gid),
-        env=test_account.env,
+        preexec_fn=demote(pkg_tests_account),
+        env=pkg_tests_account_environ,
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
         check=False,
@@ -141,8 +156,8 @@ def test_pip_non_root(shell, install_salt, test_account, extras_pypath_bin, pypa
     # Now, we should still not be able to install as non-root
     ret = subprocess.run(
         install_salt.binary_paths["pip"] + ["install", "pep8"],
-        preexec_fn=demote(test_account.uid, test_account.gid),
-        env=test_account.env,
+        preexec_fn=demote(pkg_tests_account),
+        env=pkg_tests_account_environ,
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
         check=False,

tests/pytests/pkg/integration/test_python.py

@@ -1,9 +1,8 @@
 import subprocess
+import textwrap
 
 import pytest
 
-from tests.pytests.pkg.support.helpers import TESTS_DIR
-
 
 @pytest.fixture
 def python_script_bin(install_salt):
@@ -13,13 +12,40 @@ def python_script_bin(install_salt):
     return install_salt.binary_paths["python"]
 
 
+@pytest.fixture
+def check_python_file(tmp_path):
+    script_path = tmp_path / "check_python.py"
+    script_path.write_text(
+        textwrap.dedent(
+            """
+        import sys
+
+        import salt.utils.data
+
+        user_arg = sys.argv
+
+        if user_arg[1] == "raise":
+            raise Exception("test")
+
+        if salt.utils.data.is_true(user_arg[1]):
+            sys.exit(0)
+        else:
+            sys.exit(1)
+        """
+        )
+    )
+    return script_path
+
+
 @pytest.mark.parametrize("exp_ret,user_arg", [(1, "false"), (0, "true")])
-def test_python_script(install_salt, exp_ret, user_arg, python_script_bin):
+def test_python_script(
+    install_salt, exp_ret, user_arg, python_script_bin, check_python_file
+):
     ret = install_salt.proc.run(
         *(
             python_script_bin
             + [
-                str(TESTS_DIR / "pytests" / "pkg" / "files" / "check_python.py"),
+                str(check_python_file),
                 user_arg,
             ]
         ),
@@ -32,12 +58,12 @@ def test_python_script(install_salt, exp_ret, user_arg, python_script_bin):
     assert ret.returncode == exp_ret, ret.stderr
 
 
-def test_python_script_exception(install_salt, python_script_bin):
+def test_python_script_exception(install_salt, python_script_bin, check_python_file):
     ret = install_salt.proc.run(
         *(
             python_script_bin
             + [
-                str(TESTS_DIR / "pytests" / "pkg" / "files" / "check_python.py"),
+                str(check_python_file),
                 "raise",
             ]
         ),

tests/pytests/pkg/integration/test_salt_call.py

@@ -49,11 +49,13 @@ def test_salt_call_local_sys_doc_aliases(salt_call_cli):
 
 
 @pytest.mark.skip_on_windows()
-def test_salt_call_cmd_run_id_runas(salt_call_cli, test_account, caplog):
+def test_salt_call_cmd_run_id_runas(salt_call_cli, pkg_tests_account, caplog):
     """
     Test salt-call --local cmd_run id with runas
     """
-    ret = salt_call_cli.run("--local", "cmd.run", "id", runas=test_account.username)
+    ret = salt_call_cli.run(
+        "--local", "cmd.run", "id", runas=pkg_tests_account.username
+    )
     assert "Environment could not be retrieved for user" not in caplog.text
-    assert str(test_account.uid) in ret.stdout
-    assert str(test_account.gid) in ret.stdout
+    assert str(pkg_tests_account.info.uid) in ret.stdout
+    assert str(pkg_tests_account.info.gid) in ret.stdout

tests/pytests/pkg/integration/test_salt_user.py

@@ -175,7 +175,7 @@ def test_pkg_paths(
 
 @pytest.mark.skip_if_binaries_missing("logrotate")
 def test_paths_log_rotation(
-    salt_master, salt_minion, salt_call_cli, install_salt, test_account
+    salt_master, salt_minion, salt_call_cli, install_salt, pkg_tests_account
 ):
     """
     Test the correct ownership is assigned when log rotation occurs
@@ -267,7 +267,7 @@ def test_paths_log_rotation(
                                 "file.replace",
                                 f"{install_salt.conf_dir}/master",
                                 "user: salt",
-                                f"user: {test_account.username}",
+                                f"user: {pkg_tests_account.username}",
                                 "flags=['IGNORECASE']",
                                 "append_if_not_found=True",
                             )
@@ -276,7 +276,7 @@ def test_paths_log_rotation(
                             # change ownership of appropriate paths to user
                             for _path in log_pkg_paths:
                                 chg_ownership_cmd = (
-                                    f"chown -R {test_account.username} {_path}"
+                                    f"chown -R {pkg_tests_account.username} {_path}"
                                 )
                                 ret = salt_call_cli.run(
                                     "--local", "cmd.run", chg_ownership_cmd
@@ -317,7 +317,9 @@ def test_paths_log_rotation(
                                 for _path in log_files_list:
                                     log_path = pathlib.Path(_path)
                                     assert log_path.exists()
-                                    assert log_path.owner() == test_account.username
+                                    assert (
+                                        log_path.owner() == pkg_tests_account.username
+                                    )
                                     assert log_path.stat().st_mode & 0o7777 == 0o640
 
                             # cleanup
@@ -328,7 +330,7 @@ def test_paths_log_rotation(
                                 "--local",
                                 "file.replace",
                                 f"{install_salt.conf_dir}/master",
-                                f"user: {test_account.username}",
+                                f"user: {pkg_tests_account.username}",
                                 "user: salt",
                                 "flags=['IGNORECASE']",
                                 "append_if_not_found=True",