Repository: StackStorm/st2 (language: python)

Issue:
Run more tests via Pants + pytest

Clean up import side effects in tests
When pants runs tests via pytest, it runs each file separately to facilitate fine-grained caching of test results (that can be invalidated based on the inferred deps of that test file). This revealed a variety of areas where tests rely on import-time side effects that happen when previous tests ran. So, this cleans up those side effects so that each test file can run in isolation.

In some cases, I was able to move the side effects from import-time to `setUpClass` time which feels cleaner to me.

As I identified imports with side-effects, I reviewed other places they were imported to consistently import them early with a comment explaining the dependency on import side-effects.

These commits were extracted from my wip work in #6202.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 9c7667727d..9e9a2ddf6b 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -59,7 +59,7 @@ Added
 * Continue introducing `pants <https://www.pantsbuild.org/docs>`_ to improve DX (Developer Experience)
   working on StackStorm, improve our security posture, and improve CI reliability thanks in part
   to pants' use of PEX lockfiles. This is not a user-facing addition.
-  #6118 #6141 #6133 #6120 #6181 #6183 #6200 #6237 #6229 #6240 #6241 #6244 #6251
+  #6118 #6141 #6133 #6120 #6181 #6183 #6200 #6237 #6229 #6240 #6241 #6244 #6251 #6253
   Contributed by @cognifloyd
 * Build of ST2 EL9 packages #6153
   Contributed by @amanda11
diff --git a/contrib/core/BUILD b/contrib/core/BUILD
index 7db2dc9d25..59673bd746 100644
--- a/contrib/core/BUILD
+++ b/contrib/core/BUILD
@@ -8,5 +8,9 @@ python_requirements(
 )
 
 python_sources(
-    dependencies=[":metadata"],
+    dependencies=[
+        ":metadata",
+        "./actions",
+        "./actions/send_mail:send_mail_resources",
+    ],
 )
diff --git a/contrib/core/actions/send_mail/BUILD b/contrib/core/actions/send_mail/BUILD
index f27e7c10ec..94280e6e49 100644
--- a/contrib/core/actions/send_mail/BUILD
+++ b/contrib/core/actions/send_mail/BUILD
@@ -1,4 +1,5 @@
-shell_source(
-    source="send_mail",
+st2_shell_sources_and_resources(
+    name="send_mail",
+    sources=["send_mail"],
     skip_shellcheck=True,
 )


Test patch:
diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py
index 69152a8398..668137e9cb 100644
--- a/st2common/tests/unit/services/test_packs.py
+++ b/st2common/tests/unit/services/test_packs.py
@@ -43,6 +43,7 @@
 from st2tests.fixtures.packs.orquesta_tests.fixture import (
     PACK_NAME as TEST_SOURCE_WORKFLOW_PACK,
 )
+import st2tests.config as tests_config
 
 SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER = {
     "description": "Action which injects a new trigger in the system.",
@@ -416,6 +417,8 @@ def test_exception_to_remove_resource_metadata_file(self, remove):
 class CloneActionDBAndFilesTestCase(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
+        super().setUpClass()
+        tests_config.parse_args()
         action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions")
         workflow_files_path = os.path.join(action_files_path, "workflows")
         if not os.path.isdir(action_files_path):
@@ -588,6 +591,11 @@ def test_workflows_directory_created_if_does_not_exist(self):
 
 
 class CloneActionFilesBackupTestCase(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        tests_config.parse_args()
+
     @classmethod
     def tearDownClass(cls):
         action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions")
diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py
index 496db3b0fa..f403d856b9 100644
--- a/st2common/tests/unit/test_db.py
+++ b/st2common/tests/unit/test_db.py
@@ -112,7 +112,7 @@ def tearDown(self):
     @classmethod
     def tearDownClass(cls):
         # since tearDown discconnects, dropping the database in tearDownClass
-        # fails withotu establishing a new connection.
+        # fails without establishing a new connection.
         cls._establish_connection_and_re_create_db()
         super().tearDownClass()
 
diff --git a/st2common/tests/unit/test_util_file_system.py b/st2common/tests/unit/test_util_file_system.py
index f45498702b..58f7bf6473 100644
--- a/st2common/tests/unit/test_util_file_system.py
+++ b/st2common/tests/unit/test_util_file_system.py
@@ -27,21 +27,21 @@
 
 class FileSystemUtilsTestCase(unittest.TestCase):
     def test_get_file_list(self):
+        # NB: Make sure to exclude BUILD files as pants will not include them in the sandbox,
+        #     but the BUILD files will be present if you directly run the tests.
+        basic_excludes = ["*.pyc", "__pycache__", "*BUILD"]
+
         # Standard exclude pattern
         directory = os.path.join(ST2TESTS_DIR, "policies")
         expected = [
-            "BUILD",
             "mock_exception.py",
             "concurrency.py",
             "__init__.py",
-            "meta/BUILD",
             "meta/mock_exception.yaml",
             "meta/concurrency.yaml",
             "meta/__init__.py",
         ]
-        result = get_file_list(
-            directory=directory, exclude_patterns=["*.pyc", "__pycache__"]
-        )
+        result = get_file_list(directory=directory, exclude_patterns=basic_excludes)
         # directory listings are sorted because the item order must be exact for assert
         # to validate equivalence.  Directory item order doesn't matter in general and may
         # even change on different platforms or locales.
@@ -55,7 +55,7 @@ def test_get_file_list(self):
             "meta/__init__.py",
         ]
         result = get_file_list(
-            directory=directory, exclude_patterns=["*.pyc", "*.yaml", "*BUILD"]
+            directory=directory, exclude_patterns=["*.yaml"] + basic_excludes
         )
         # directory listings are sorted because the item order must be exact for assert
         # to validate equivalence.  Directory item order doesn't matter in general and may
diff --git a/st2tests/st2tests/policies/meta/BUILD b/st2tests/st2tests/policies/meta/BUILD
index 0700f34cd5..53560669a3 100644
--- a/st2tests/st2tests/policies/meta/BUILD
+++ b/st2tests/st2tests/policies/meta/BUILD
@@ -1,3 +1,9 @@
 resources(
-    sources=["*.yaml"],
+    sources=[
+        "*.yaml",
+        # pants ignores empty __init__.py files.
+        # However, the tests for st2common.util.file_system need it to be present,
+        # so we treat it as a resource instead of a python source file.
+        "__init__.py",
+    ],
 )