Repository: ukaea/bom_analysis

Issue:
correctly remove temporary directory run log handler

Discussion:

Patch:
diff --git a/bom_analysis/base.py b/bom_analysis/base.py
index 1daa145..5c4bc97 100644
--- a/bom_analysis/base.py
+++ b/bom_analysis/base.py
@@ -4,6 +4,7 @@ from pathlib import Path
 import os
 from getpass import getpass
 from typing import Union
+import pprint
 
 import json
 import numpy as np
@@ -280,6 +281,9 @@ class MetaConfig(type):
         """
         cls._temp_dir = value
         change_handler(f"{value}/run.log")
+        run_log.info(
+            f"Configuration Details\n\n{pprint.pformat(cls.to_dict(), indent=4)}"
+        )
 
     @property
     def plot_dir(cls):
@@ -407,6 +411,7 @@ class BaseConfigMethods:
                 isinstance(val, property),
                 isinstance(val, classmethod),
                 key in vars(BaseClass).keys(),
+                key == "_login_details",
             ]
             if not any(check):
                 variables[key] = val
diff --git a/bom_analysis/utils.py b/bom_analysis/utils.py
index 6ade84d..f28c8c7 100644
--- a/bom_analysis/utils.py
+++ b/bom_analysis/utils.py
@@ -12,7 +12,15 @@ import json
 import numpy as np
 import pandas as pd
 
-from bom_analysis import ureg, run_log, nice_format, info_handler, Q_
+from bom_analysis import (
+    ureg,
+    run_log,
+    nice_format,
+    info_handler,
+    base_handler,
+    console_handler,
+    Q_,
+)
 
 
 def __init__(self, inherited_classes):
@@ -137,12 +145,15 @@ def change_handler(new_path):
     ----------
     new_path : str
         The new path for the run log to be stored."""
-    run_log.removeHandler(info_handler)
+    run_log.handlers.clear()
     info_handler.flush()
     info_handler.close()
     new_info_handler = logging.FileHandler(Path(new_path), "w")
     new_info_handler.setLevel(logging.INFO)
     new_info_handler.setFormatter(nice_format)
+
+    run_log.addHandler(base_handler)
+    run_log.addHandler(console_handler)
     run_log.addHandler(new_info_handler)
 
 


Test patch:
diff --git a/tests/test_Base.py b/tests/test_Base.py
index ba7e6d5..7e6e56e 100644
--- a/tests/test_Base.py
+++ b/tests/test_Base.py
@@ -182,6 +182,18 @@ class TestConfig(unittest.TestCase):
         con = Config.to_dict()
         assert con["b"] == "foo"
 
+    def test_to_dict_without_login(self):
+        """tests whether to_dict works for different cases"""
+        config = {"a": 1, "b": "foo", "c": "bar"}
+        Config.define_config(config_dict=config)
+        Config._login_details = {
+            "username": "secret",
+            "password": "secret",
+            "domain": "secret",
+        }
+        dump = Config.to_dict()
+        assert "_login_details" not in dump
+
     def test_assignment_to_property(self):
         Config.restrict_param = "hello"
         assert Config.restrict_param == "hello"
diff --git a/tests/test_logging.py b/tests/test_logging.py
index a7fae83..fdbccdb 100644
--- a/tests/test_logging.py
+++ b/tests/test_logging.py
@@ -1,10 +1,15 @@
 import unittest
+from unittest.mock import patch
+import tempfile
+
+import pytest
 
 from bom_analysis import run_log
 from bom_analysis.utils import change_handler
 from bom_analysis.base import BaseConfig
 
 
+@pytest.mark.unittest
 class LoggingTest(unittest.TestCase):
     def setUp(self):
         change_handler("./run.log")
@@ -44,25 +49,42 @@ class LoggingTest(unittest.TestCase):
             content = f.readline()
         assert content == "INFO: In ./temp/run.log\n"
 
-    def test_change_location_config(self):
+    @patch("bom_analysis.BaseConfig.to_dict")
+    def test_change_location_config(self, td):
+        td.return_value = {}
         run_log.info("Again in ./run.log")
         with open("./run.log", "r") as f:
-            content = f.readline()
+            content = f.readlines()
 
-        assert content == "INFO: Again in ./run.log\n"
+        assert content[-2] == "INFO: Again in ./run.log\n"
 
         BaseConfig.temp_dir = "./temp/"
 
         run_log.info("Again in ./temp/run.log")
         with open("./temp/run.log", "r") as f:
-            content = f.readline()
-        assert content == "INFO: Again in ./temp/run.log\n"
+            content = f.readlines()
+        assert content[-2] == "INFO: Again in ./temp/run.log\n"
+        with tempfile.TemporaryDirectory(prefix="temp_", dir="./temp/") as tmp_dir:
+            BaseConfig.temp_dir = tmp_dir
+
+            run_log.info("final test in a new dir")
+            with open(f"{tmp_dir}/run.log", "r") as f:
+                content = f.readlines()
+            assert content[-2] == "INFO: final test in a new dir\n"
+            BaseConfig.temp_dir = "./"
+        with open("./temp/run.log", "r") as f:
+            content = f.readlines()
+
+        assert content[-2] == "INFO: Again in ./temp/run.log\n"
 
         with open("./base.log", "r") as f:
             base_content = f.readlines()
-        assert base_content[-2][26::] == "INFO in test_logging: Again in ./run.log\n"
         assert (
-            base_content[-1][26::] == "INFO in test_logging: Again in ./temp/run.log\n"
+            base_content[-4][26::] == "INFO in test_logging: final test in a new dir\n"
+        )
+        assert base_content[-12][26::] == "INFO in test_logging: Again in ./run.log\n"
+        assert (
+            base_content[-8][26::] == "INFO in test_logging: Again in ./temp/run.log\n"
         )