From 4e59ec919c64e44ec01787d768dbd7ab08c9072d Mon Sep 17 00:00:00 2001
From: Cleber Rosa <crosa@redhat.com>
Date: Mon, 26 Nov 2018 13:49:09 -0500
Subject: [PATCH] Job: clean up "dry-run" jobs by default

In an effort to make jobs more self contained, and given that dry-run
jobs created temporary content that is not supposed to be kept after
the job is run, this introduces the following changes:

 * Jobs with "dry-run" enabled will now, by default, cleanup their
   own data.  Basically, this means that the "base log directory"
   created during job initialization, and that for non "dry-run"
   jobs is usually "~/avocado/job-results", will also be removed
   at the job's "cleanup()" phase.

 * A new "--dry-run-no-cleanup" command line option, that only makes
   sense when jobs with dry-run enabled start to clean up data for
   themselves

 * Because some result plugins will, by default, write to the job
   results directory, results plugins are now run before a job cleanup
   is done.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 avocado/core/job.py                | 16 ++++++++++++++++
 avocado/plugins/run.py             | 14 +++++++++-----
 selftests/functional/test_basic.py |  7 ++++---
 selftests/unit/test_job.py         |  6 ++----
 4 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/avocado/core/job.py b/avocado/core/job.py
index 61ad4856..2baa47c8 100644
--- a/avocado/core/job.py
+++ b/avocado/core/job.py
@@ -573,6 +573,22 @@ class Job(object):
         self.__stop_job_logging()
         if not self.__keep_tmpdir and os.path.exists(self.tmpdir):
             shutil.rmtree(self.tmpdir)
+        cleanup_conditionals = (
+            getattr(self.args, "dry_run", False),
+            not getattr(self.args, "dry_run_no_cleanup", False)
+        )
+        if all(cleanup_conditionals):
+            # Also clean up temp base directory created because of the dry-run
+            base_logdir = getattr(self.args, "base_logdir", None)
+            if base_logdir is not None:
+                try:
+                    FileNotFoundError
+                except NameError:
+                    FileNotFoundError = OSError   # pylint: disable=W0622
+                try:
+                    shutil.rmtree(base_logdir)
+                except FileNotFoundError:
+                    pass
 
 
 class TestProgram(object):
diff --git a/avocado/plugins/run.py b/avocado/plugins/run.py
index b4575002..eb8165c2 100644
--- a/avocado/plugins/run.py
+++ b/avocado/plugins/run.py
@@ -76,6 +76,10 @@ class Run(CLICmd):
                             help="Instead of running the test only "
                             "list them and log their params.")
 
+        parser.add_argument("--dry-run-no-cleanup", action="store_true",
+                            help="Do not automatically clean up temporary "
+                            "directories used by dry-run", default=False)
+
         parser.add_argument('--force-job-id', dest='unique_job_id',
                             type=str, default=None,
                             help='Forces the use of a particular job ID. Used '
@@ -237,9 +241,9 @@ class Run(CLICmd):
                 # Run JobPost plugins
                 pre_post_dispatcher.map_method('post', job_instance)
 
-        result_dispatcher = ResultDispatcher()
-        if result_dispatcher.extensions:
-            result_dispatcher.map_method('render',
-                                         job_instance.result,
-                                         job_instance)
+            result_dispatcher = ResultDispatcher()
+            if result_dispatcher.extensions:
+                result_dispatcher.map_method('render',
+                                             job_instance.result,
+                                             job_instance)
         return job_run
diff --git a/selftests/functional/test_basic.py b/selftests/functional/test_basic.py
index 300b402e..52c67f55 100644
--- a/selftests/functional/test_basic.py
+++ b/selftests/functional/test_basic.py
@@ -495,9 +495,10 @@ class RunnerOperationTest(unittest.TestCase):
             avocado_process.wait()
 
     def test_dry_run(self):
-        cmd = ("%s run --sysinfo=off passtest.py failtest.py "
-               "gendata.py --json - --mux-inject foo:1 bar:2 baz:3 foo:foo:a"
-               " foo:bar:b foo:baz:c bar:bar:bar --dry-run" % AVOCADO)
+        cmd = ("%s run --sysinfo=off --dry-run --dry-run-no-cleanup --json - "
+               "--mux-inject foo:1 bar:2 baz:3 foo:foo:a "
+               "foo:bar:b foo:baz:c bar:bar:bar "
+               "-- passtest.py failtest.py gendata.py " % AVOCADO)
         result = json.loads(process.run(cmd).stdout_text)
         debuglog = result['debuglog']
         log = genio.read_file(debuglog)
diff --git a/selftests/unit/test_job.py b/selftests/unit/test_job.py
index f51d2625..7248e872 100644
--- a/selftests/unit/test_job.py
+++ b/selftests/unit/test_job.py
@@ -232,12 +232,10 @@ class JobTest(unittest.TestCase):
     def test_job_dryrun_no_base_logdir(self):
         args = argparse.Namespace(dry_run=True)
         self.job = job.Job(args)
-        self.job.setup()
-        try:
+        with self.job:
             self.assertTrue(os.path.isdir(self.job.logdir))
             self.assertTrue(os.path.isfile(os.path.join(self.job.logdir, 'id')))
-        finally:
-            shutil.rmtree(self.job.args.base_logdir)
+        self.assertFalse(os.path.isdir(self.job.logdir))
 
     def tearDown(self):
         data_dir._tmp_tracker.unittest_refresh_dir_tracker()
-- 
GitLab