Tricky problem in python unit/sys testing regarding temporary directories

[ekluzek]

Brief summary of bug

This is a problem we keep running into with our python testing. Some of the tests have the Setup method create a temporary directory that is then destroyed at the TearDown step. The problem happens if the current working directory is the directory that is destroyed. The test that does this actually works fine.

But, if the next test stays in that directory (that is now gone) and tries to do anything -- weird system errors occur. And that next test fails -- but NOT do to anything in that test. It's all an artifact of the previous test. And since successful tests don't show any output you don't have any information on the test that causes the problem. Then you try to find non existing problems in the test that fails -- but since there isn't any problem there you don't find any. And it becomes frustrating.

Here's some example commits that deal with this:

747eda6
0b066be

I know I've done work on this as well.

[ekluzek]

What we've been doing to mitigate this problem are changes like this:

--- a/python/ctsm/test/joblauncher/test_unit_job_launcher_no_batch.py
+++ b/python/ctsm/test/joblauncher/test_unit_job_launcher_no_batch.py
@@ -21,9 +21,11 @@ class TestJobLauncherNoBatch(unittest.TestCase):
     """Tests of job_launcher_no_batch"""
 
     def setUp(self):
+        self._previous_dir = os.getcwd()
         self._testdir = tempfile.mkdtemp()
 
     def tearDown(self):
+        os.chdir(self._previous_dir )
         shutil.rmtree(self._testdir, ignore_errors=True)
 
     def assertFileContentsEqual(self, expected, filepath, msg=None):

So the setup is just going to the previous directory. The problem with this is that if the previous directory is a temporary directory that was deleted, this is still problematic. I think what we should do is to set the directory to go to as the python parent directory. Then we always know that's a safe place to go to.

[ekluzek]

I'm also thinking we should have a little utility to handle this situation so developers don't have to think about this and just use the standard tempdir setup and tear down code. There could also be checking that you aren't removing the directory you are in for example.

[wwieder]

@samsrabin wonders if this is an issue with the makefile ghost again? @ekluzek doesn't think this is the issue.

See this PR

[samsrabin]

Looks like I had it backward. I found my notes from the standup where we discussed this (2023-07-17), and it turns out make test was the method that actually worked:

The mentioned Discussion is #2059.

[ekluzek]

Here's another instance of a commit regarding this on the ctsm5.2 branch:

59d45d0

[ekluzek]

This came up again in the b4b-dev merge. See this comment...

#2385 (comment)