preserve-cwd only when preserve-cwd is provided (bugfix) (#2352)

* preserve-cwd only when preserve-cwd is provided

Snaps services cwd is calculated by snapd and depends on the user. Using
the service cwd as the job cwd present one challenge, if the job is a
user job it may not have access to that snap cwd (that may be in
/root/snap/...). Additionally I believe that preserving cwd always in
snaps was something done in the past because `/tmp` is private for
snaps. For now `/var/tmp` is not, and we use this quite a lot, so I'll
use that

* Update test to the new nominal behaviour

Author: Hook25

checkbox-ng/plainbox/impl/execution.py

@@ -426,14 +426,17 @@ def get_proper_job_cwd(self, job):
         :returns:
             Pathname of the new directory
         """
-        if "preserve-cwd" in job.get_flag_set() or os.getenv("SNAP"):
+        if "preserve-cwd" in job.get_flag_set():
             yield os.getcwd()
             return
         # Create a nest for all the private executables needed for execution
         prefix = "cwd-"
         suffix = ".{}".format(job.checksum)
         try:
-            with tempfile.TemporaryDirectory(suffix, prefix) as cwd_dir:
+            # use /var/tmp because in snaps /tmp is private to the snap
+            with tempfile.TemporaryDirectory(
+                suffix, prefix, "/var/tmp"
+            ) as cwd_dir:
                 os.chmod(cwd_dir, 0o777)
                 logger.debug(
                     _("Job temporary current working directory: %s"), cwd_dir

checkbox-ng/plainbox/impl/test_execution.py

@@ -334,13 +334,30 @@ def test_get_proper_job_cwd_preserve_cwd(self, os_cwd_mock):
 
     @mock.patch("os.getcwd")
     @mock.patch("os.getenv")
-    def test_get_proper_job_cwd_snap(self, os_getenv_mock, os_cwd_mock):
+    @mock.patch("os.chmod")
+    def test_get_proper_job_cwd_snap(
+        self, chmod_mock, os_getenv_mock, os_cwd_mock
+    ):
         self_mock = mock.Mock()
         job_mock = mock.Mock()
         job_mock.get_flag_set.return_value = {}
         os_getenv_mock.return_value = "/snap/checkbox24"
-        with UnifiedRunner.get_proper_job_cwd(self_mock, job_mock) as cwd:
-            self.assertEqual(cwd, os_cwd_mock())
+
+        class TemporaryDirectoryMock:
+            def __init__(self, suffix, prefix, dir, *args, **kwargs):
+                self.dir = dir
+
+            def __enter__(self):
+                return self.dir + "/some"
+
+            def __exit__(self, *args): ...
+
+        with mock.patch(
+            "tempfile.TemporaryDirectory", new=TemporaryDirectoryMock
+        ) as tmp:
+            with UnifiedRunner.get_proper_job_cwd(self_mock, job_mock) as cwd:
+                self.assertTrue(str(cwd).startswith("/var/tmp"))
+            self.assertTrue(self_mock._check_leftovers.called)
 
 
 class TestDangerousNsenter(TestCase):