From f5b0f91d33ab2878a779f9f6e0f1cd730e2e465d Mon Sep 17 00:00:00 2001
From: Thibault Saunier <tsaunier@igalia.com>
Date: Tue, 24 Sep 2024 09:48:54 -0300
Subject: [PATCH] validate: launcher: Remove log files for passing tests by
 default

Adding an option to keep them no matter what.
Log files are often pretty large and keeping them around can be annoying,
usually people won't look at logs files for passing tests, and we do not
even print them out.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7700>
---
 .../validate/launcher/baseclasses.py          | 53 ++++++++++++++-----
 .../gst-devtools/validate/launcher/main.py    |  7 +++
 2 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/subprojects/gst-devtools/validate/launcher/baseclasses.py b/subprojects/gst-devtools/validate/launcher/baseclasses.py
index c3186fff03..714ec73553 100644
--- a/subprojects/gst-devtools/validate/launcher/baseclasses.py
+++ b/subprojects/gst-devtools/validate/launcher/baseclasses.py
@@ -192,6 +192,14 @@ class Test(Loggable):
 
         self.clean()
 
+    def remove_logs(self):
+        for logfile in set([self.logfile]) | self.extra_logfiles:
+            try:
+                os.remove(logfile)
+                self.info("Removed %s after test passed.", logfile)
+            except FileNotFoundError:
+                pass
+
     def _generate_expected_issues(self):
         return ''
 
@@ -309,14 +317,18 @@ class Test(Loggable):
             for logfile in self.extra_logfiles:
                 # Only copy over extra logfile content if it's below a certain threshold
                 # Avoid copying gigabytes of data if a lot of debugging is activated
-                if os.path.getsize(logfile) < 500 * 1024:
-                    self.out.write('\n\n## %s:\n\n```\n%s\n```\n' % (
-                        os.path.basename(logfile), self.get_extra_log_content(logfile))
-                    )
-                else:
-                    self.out.write('\n\n## %s:\n\n**Log file too big.**\n  %s\n\n Check file content directly\n\n' % (
-                        os.path.basename(logfile), logfile)
-                    )
+                try:
+                    if os.path.getsize(logfile) < 500 * 1024:
+                        self.out.write('\n\n## %s:\n\n```\n%s\n```\n' % (
+                            os.path.basename(logfile), self.get_extra_log_content(logfile))
+                        )
+                    else:
+                        self.out.write('\n\n## %s:\n\n**Log file too big.**\n  %s\n\n Check file content directly\n\n' % (
+                            os.path.basename(logfile), logfile)
+                        )
+                except FileNotFoundError as e:
+                    self.info(f"Failed to copy logfile {logfile}: {e}")
+                    pass
 
             if self.rr_logdir:
                 self.out.write('\n\n## rr trace:\n\n```\nrr replay %s/latest-trace\n```\n' % (
@@ -785,7 +797,11 @@ class Test(Loggable):
         self.logfile = shutil.copy(self.logfile, path)
         extra_logs = []
         for logfile in self.extra_logfiles:
-            extra_logs.append(shutil.copy(logfile, path))
+            try:
+                extra_logs.append(shutil.copy(logfile, path))
+            except FileNotFoundError as e:
+                self.info(f"Failed to copy logfile {logfile}: {e}")
+                pass
         self.extra_logfiles = extra_logs
 
     def test_end(self, retry_on_failures=False):
@@ -927,6 +943,7 @@ class GstValidateTest(Test):
         self.media_descriptor = media_descriptor
         self.server = None
         self.criticals = []
+        self.dotfilesdir = None
 
         override_path = self.get_override_file(media_descriptor)
         if override_path:
@@ -1050,17 +1067,21 @@ class GstValidateTest(Test):
                 pass
 
         if not subproc_env.get('GST_DEBUG_DUMP_DOT_DIR'):
-            dotfilesdir = os.path.join(self.options.logsdir,
-                                self.classname.replace(".", os.sep) + '.pipelines_dot_files')
-            mkdir(dotfilesdir)
-            subproc_env['GST_DEBUG_DUMP_DOT_DIR'] = dotfilesdir
+            self.dotfilesdir = os.path.splitext(self.logfile)[0] + '.pipelines_dot_files'
+            mkdir(self.dotfilesdir)
+            subproc_env['GST_DEBUG_DUMP_DOT_DIR'] = self.dotfilesdir
             if CI_ARTIFACTS_URL:
-                dotfilesurl = CI_ARTIFACTS_URL + os.path.relpath(dotfilesdir,
+                dotfilesurl = CI_ARTIFACTS_URL + os.path.relpath(self.dotfilesdir,
                                                                  self.options.logsdir)
                 subproc_env['GST_VALIDATE_DEBUG_DUMP_DOT_URL'] = dotfilesurl
 
         return subproc_env
 
+    def remove_logs(self):
+        super().remove_logs()
+        if self.dotfilesdir:
+            shutil.rmtree(self.dotfilesdir, ignore_errors=True)
+
     def clean(self):
         Test.clean(self)
         self.reports = []
@@ -2299,6 +2320,10 @@ class _TestsLauncher(Loggable):
                     total_num_tests=total_num_tests)
                 if to_report:
                     self.reporter.after_test(test)
+
+                if res == Result.PASSED and not self.options.keep_logs:
+                    test.remove_logs()
+
                 if self.start_new_job(tests_left):
                     jobs_running += 1
 
diff --git a/subprojects/gst-devtools/validate/launcher/main.py b/subprojects/gst-devtools/validate/launcher/main.py
index 6649003f11..cec7a45538 100644
--- a/subprojects/gst-devtools/validate/launcher/main.py
+++ b/subprojects/gst-devtools/validate/launcher/main.py
@@ -221,6 +221,7 @@ class LauncherConfig(Loggable):
         self.num_jobs = max(multiprocessing.cpu_count(), 1)
         self.dest = None
         self._using_default_paths = False
+        self.keep_logs = False
         # paths passed with --media-path, and not defined by a testsuite
         self.user_paths = []
         self.paths = []
@@ -267,6 +268,9 @@ class LauncherConfig(Loggable):
             self.debug = True
             self.num_jobs = 1
 
+        if self.xunit_file:
+            self.keep_logs = True
+
         # other output directories
         if self.logsdir in ['stdout', 'stderr']:
             # Allow -l stdout/stderr to work like -rl stdout/stderr
@@ -520,6 +524,9 @@ class LauncherConfig(Loggable):
                             help="Disable retrying on failure, event for known to be flaky tests.")
         parser.add_argument('--html', dest="html", action="store_true",
                             help="Write logs as html")
+        parser.add_argument("--keep-logs", dest="keep_logs",
+                            action="store_true",
+                            help="Keep the logs in the output directory on success, by default logs are removed unless the test passes")
         dir_group = parser.add_argument_group(
             "Directories and files to be used by the launcher")
         dir_group.add_argument("-M", "--main-dir", dest="main_dir",