From 44be35d2792a251bb5aff155e32bf18138e5e9f7 Mon Sep 17 00:00:00 2001
From: Dima Gerasimov <karlicoss@gmail.com>
Date: Sun, 25 Aug 2024 18:51:34 +0100
Subject: [PATCH] tests: move test checkign for my_config handling to
 core/tests/test_config.py

allows to remove the hacky reset_modules thing from setup fixture
---
 my/core/tests/test_config.py | 47 ++++++++++++++++++++++++++++++++++++
 tests/config.py              | 41 ++-----------------------------
 2 files changed, 49 insertions(+), 39 deletions(-)

diff --git a/my/core/tests/test_config.py b/my/core/tests/test_config.py
index c76f5d9c..0da6dd81 100644
--- a/my/core/tests/test_config.py
+++ b/my/core/tests/test_config.py
@@ -3,6 +3,7 @@
 """
 
 import sys
+import os
 from pathlib import Path
 
 import pytest
@@ -13,6 +14,10 @@
 from my.core import notnone
 from my.demo import items, make_config
 
+from .common import tmp_environ_set
+
+# TODO would be nice to randomize test order here to catch various config issues
+
 
 # run the same test multiple times to make sure there are not issues with import order etc
 @pytest.mark.parametrize('run_id', ['1', '2'])
@@ -120,6 +125,48 @@ class user_config:
     sys.modules.pop('external.submodule', None)
 
 
+@pytest.mark.parametrize('run_id', ['1', '2'])
+def test_my_config_env_variable(tmp_path: Path, run_id: str) -> None:
+    """
+    Tests handling of MY_CONFIG variable
+    """
+
+    # ugh. so by this point, my.config is already loaded (default stub), so we need to unload it
+    sys.modules.pop('my.config', None)
+    # but my.config itself relies on my.core.init hook, so unless it's reloaded too it wouldn't help
+    sys.modules.pop('my.core', None)
+    sys.modules.pop('my.core.init', None)
+    # it's a bit of a mouthful of course, but in most cases MY_CONFIG would be set once
+    #  , and before hpi runs, so hopefully it's not a huge deal
+    cfg_dir = tmp_path / 'my'
+    cfg_file = cfg_dir / 'config.py'
+    cfg_dir.mkdir()
+
+    cfg_file.write_text(
+        f'''
+# print("IMPORTING CONFIG {run_id}")
+class demo:
+    username = 'xxx_{run_id}'
+    data_path = '{tmp_path}{os.sep}*.json'
+'''
+    )
+
+    with tmp_environ_set('MY_CONFIG', str(tmp_path)):
+        [item1, item2] = items()
+        assert item1.username == f'xxx_{run_id}'
+        assert item2.username == f'xxx_{run_id}'
+
+        # sigh.. so this is cached in sys.path
+        #  so it takes precedence later during next import, not giving the MY_CONFIG hook
+        #  (imported from builtin my.config) to kick in
+        sys.path.remove(str(tmp_path))
+
+        # FIXME ideally this shouldn't be necessary?
+        #  remove this after we fixup my.tests.reddit and my.tests.commits
+        #  (they were failing ci when running all tests)
+        sys.modules.pop('my.config', None)
+
+
 @pytest.fixture(autouse=True)
 def prepare_data(tmp_path: Path):
     (tmp_path / 'data.json').write_text(
diff --git a/tests/config.py b/tests/config.py
index 101f7df4..acfe1f1d 100644
--- a/tests/config.py
+++ b/tests/config.py
@@ -1,6 +1,7 @@
 from pathlib import Path
 
 
+# TODO move this somewhere else -- there are more specific tests covering this now
 def test_dynamic_configuration(notes: Path) -> None:
     import pytz
     from types import SimpleNamespace as NS
@@ -26,42 +27,11 @@ def test_dynamic_configuration(notes: Path) -> None:
 import pytest
 
 
-def test_environment_variable(tmp_path: Path) -> None:
-    cfg_dir  = tmp_path / 'my'
-    cfg_file = cfg_dir / 'config.py'
-    cfg_dir.mkdir()
-    cfg_file.write_text('''
-class feedly:
-    pass
-class just_for_test:
-    pass
-''')
-
-    import os
-    oenv = dict(os.environ)
-    try:
-        os.environ['MY_CONFIG'] = str(tmp_path)
-        # should not raise at least
-        import my.rss.feedly
-
-        import my.config as c
-        assert hasattr(c, 'just_for_test')
-    finally:
-        os.environ.clear()
-        os.environ.update(oenv)
-
-    import sys
-    # TODO wtf??? doesn't work without unlink... is it caching something?
-    cfg_file.unlink()
-    del sys.modules['my.config'] # meh..
-
-    import my.config as c
-    assert not hasattr(c, 'just_for_test')
-
 
 from dataclasses import dataclass
 
 
+# TODO this test should probs be deprecated? it's more of a documentation?
 def test_user_config() -> None:
     from my.core.common import classproperty
     class user_config:
@@ -117,10 +87,3 @@ def notes(tmp_path: Path):
         yield ndir
     finally:
         pass
-
-
-@pytest.fixture(autouse=True)
-def prepare():
-    from my.tests.common import reset_modules
-    reset_modules()
-    yield