[dexter] Don't generate results files by default

[Orlando]

Summary

Dexter saves various files to a new results directory each time it is run (including when it's run by lit tests) and there isn't a way to opt-out. This patch reconfigures the behaviour to be opt-in by removing the default --results-directory location. Now results are only saved if --results-directory is specified.

[jmorse]

LGTM with query; More restructuring to avoid more indentation would be nice, but here we are ._.

cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/clang-opt-bisect/clang-opt-bisect-results.cpp

13

Seeing rm -rf in any test makes me twitch; would we be able to explicitly delete each file in the results dir, then call rmdir? That at least allows for non-destructive-failures when %t points somewhere ever so slightly unexpected.

[Orlando]

+ addressed inline comment

cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/clang-opt-bisect/clang-opt-bisect-results.cpp

13

Like this?

[Orlando]

LGTM with query; More restructuring to avoid more indentation would be nice, but here we are ._.

I agree, but I wanted to just stop the (disk-space) bleeding. It looks like some pickles are getting saved in there too (something else that should probably be looked into), so these results dirs get surprisingly large.

[jmorse]

cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/clang-opt-bisect/clang-opt-bisect-results.cpp

13

Yup; consider it damage limitation for when weird things happen.

cross-project-tests/debuginfo-tests/dexter/dex/tools/TestToolBase.py

def add_tool_arguments(self, parser, defaults):
             default=os.path.abspath(
                 os.path.join(get_root_directory(), '..', 'tests')),
             help='directory containing test(s)')
 
         parser.add_argument(
             '--results-directory',
             type=str,
             metavar='<directory>',
-            default=os.path.abspath(
-                os.path.join(get_root_directory(), '..', 'results',
-                             datetime.now().strftime('%Y-%m-%d-%H%M-%S'))),
-            help='directory to save results')
+            default=None,
+            help='directory to save results (default: none)')
 
     def handle_options(self, defaults):
         options = self.context.options
 
         if not options.builder and (options.cflags or options.ldflags):
             warn(self.context, '--cflags and --ldflags will be ignored when not'
                                ' using --builder')
 
def handle_options(self, defaults):
 
         options.test_path = os.path.abspath(options.test_path)
         options.test_path = os.path.normcase(options.test_path)
         if not os.path.isfile(options.test_path) and not os.path.isdir(options.test_path):
             raise Error(
                 '<d>could not find test path</> <r>"{}"</>'.format(
                     options.test_path))
 
+        if options.results_directory:
             options.results_directory = os.path.abspath(options.results_directory)
             if not os.path.isdir(options.results_directory):
                 try:
                     os.makedirs(options.results_directory, exist_ok=True)
                 except OSError as e:
                     raise Error(
                         '<d>could not create directory</> <r>"{}"</> <y>({})</>'.
                         format(options.results_directory, e.strerror))
 
     def go(self) -> ReturnCode:  # noqa
         options = self.context.options
 
         options.executable = os.path.join(
             self.context.working_directory.path, 'tmp.exe')
 
         # Test files contain dexter commands.

cross-project-tests/debuginfo-tests/dexter/dex/tools/clang_opt_bisect/Tool.py

def _run_test(self, test_name):  # noqa
 
             if options.verbose:
                 self.context.o.auto(heuristic_verbose_output)
 
             steps_str = str(steps)
             steps_changed = steps_str != prev_steps_str
             prev_steps_str = steps_str
 
-            # If this is the first pass, or something has changed, write a text
-            # file containing verbose information on the current status.
-            if current_limit == 0 or score_difference or steps_changed:
+            # If a results directory has been specified and this is the first
+            # pass or something has changed, write a text file containing
+            # verbose information on the current status.
+            if options.results_directory and (current_limit == 0 or
+                                              score_difference or
+                                              steps_changed):
                 file_name = '-'.join(
                     str(s) for s in [
                         'status', test_name, '{{:0>{}}}'.format(
                             len(str(overall_limit))).format(current_limit),
                         '{:.4f}'.format(heuristic.score).replace(
                             '.', '_'), pass_info[1]
                     ] if s is not None)
 
def _run_test(self, test_name):  # noqa
 
             if pass_info[1]:
                 self._all_bisect_pass_summary[pass_info[1]].append(
                     score_difference)
 
                 current_bisect_pass_summary[pass_info[1]].append(
                     score_difference)
 
+        if options.results_directory:
             per_pass_score_path = os.path.join(
                 options.results_directory,
                 '{}-per_pass_score.csv'.format(test_name))
 
             with open(per_pass_score_path, mode='w', newline='') as fp:
                 writer = csv.writer(fp, delimiter=',')
                 writer.writerow(['Source File', 'Pass', 'Score'])
 
                 for path, pass_, score in per_pass_score:
                     writer.writerow([path, pass_, score])
             self.context.o.blue('wrote "{}"\n'.format(per_pass_score_path))
 
             pass_summary_path = os.path.join(
                 options.results_directory, '{}-pass-summary.csv'.format(test_name))
 
             self._write_pass_summary(pass_summary_path,
                                      current_bisect_pass_summary)
 
     def _handle_results(self) -> ReturnCode:
         options = self.context.options
+        if options.results_directory:
             pass_summary_path = os.path.join(options.results_directory,
                                              'overall-pass-summary.csv')
 
             self._write_pass_summary(pass_summary_path,
                                      self._all_bisect_pass_summary)
         return ReturnCode.OK
 
     def _clang_opt_bisect_build(self, opt_bisect_limits):
         options = self.context.options
         compiler_options = [
             '{} -mllvm -opt-bisect-limit={}'.format(options.cflags,
                                                     opt_bisect_limit)
             for opt_bisect_limit in opt_bisect_limits

cross-project-tests/debuginfo-tests/dexter/dex/tools/test/Tool.py

def _get_results_basename(self, test_name):
               yield y
         all_components = reversed([x for x in splitall(test_name)])
         return '_'.join(all_components)
 
     def _get_results_path(self, test_name):
         """Returns the path to the test results directory for the test denoted
         by test_name.
         """
+        assert self.context.options.results_directory != None
         return os.path.join(self.context.options.results_directory,
                             self._get_results_basename(test_name))
 
     def _get_results_text_path(self, test_name):
         """Returns path results .txt file for test denoted by test_name.
         """
         test_results_path = self._get_results_path(test_name)
         return '{}.txt'.format(test_results_path)
 
     def _get_results_pickle_path(self, test_name):
         """Returns path results .dextIR file for test denoted by test_name.
         """
         test_results_path = self._get_results_path(test_name)
         return '{}.dextIR'.format(test_results_path)
 
     def _record_steps(self, test_name, steps):
         """Write out the set of steps out to the test's .txt and .json
-        results file.
+        results file if a results directory has been specified.
         """
+        if self.context.options.results_directory:
             output_text_path = self._get_results_text_path(test_name)
             with open(output_text_path, 'w') as fp:
                 self.context.o.auto(str(steps), stream=Stream(fp))
 
             output_dextIR_path = self._get_results_pickle_path(test_name)
             with open(output_dextIR_path, 'wb') as fp:
                 pickle.dump(steps, fp, protocol=pickle.HIGHEST_PROTOCOL)
 
     def _record_score(self, test_name, heuristic):
-        """Write out the test's heuristic score to the results .txt file.
+        """Write out the test's heuristic score to the results .txt file
+        if a results directory has been specified.
         """
+        if self.context.options.results_directory:
             output_text_path = self._get_results_text_path(test_name)
             with open(output_text_path, 'a') as fp:
                 self.context.o.auto(heuristic.verbose_output, stream=Stream(fp))
 
     def _record_test_and_display(self, test_case):
         """Output test case to o stream and record test case internally for
         handling later.
         """
         self.context.o.auto(test_case)
         self._test_cases.append(test_case)
 
def _handle_results(self) -> ReturnCode:
                 score = test_case.score
                 if not test_case.error and not math.isnan(score):
                     score_sum += test_case.score
                     num_tests += 1
 
             if num_tests != 0:
                 print("@avg: ({:.4f})".format(score_sum/num_tests))
 
+        has_failed = lambda test: test.score < options.fail_lt or test.error
+        if any(map(has_failed, self._test_cases)):
+            return_code = ReturnCode.FAIL
+
+        if options.results_directory:
             summary_path = os.path.join(options.results_directory, 'summary.csv')
             with open(summary_path, mode='w', newline='') as fp:
                 writer = csv.writer(fp, delimiter=',')
                 writer.writerow(['Test Case', 'Score', 'Error'])
 
                 for test_case in self._test_cases:
-                if (test_case.score < options.fail_lt or
-                        test_case.error is not None):
-                    return_code = ReturnCode.FAIL
-
                     writer.writerow([
                         test_case.name, '{:.4f}'.format(test_case.score),
                         test_case.error
                     ])
 
         return return_code

cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/clang-opt-bisect/clang-opt-bisect-results.cpp

+// Purpose:
+//     Check the `clang-opt-bisect` tool runs with --results-directory.
+//
+// RUN: true
+// RUN: %dexter_base clang-opt-bisect \
+// RUN:     --debugger %dexter_regression_test_debugger \
+// RUN:     --builder %dexter_regression_test_builder \
+// RUN:     --cflags "%dexter_regression_test_cflags" \
+// RUN:     --ldflags "%dexter_regression_test_ldflags" \
+// RUN:     --results-directory=%t \
+// RUN:     -- %s \
+// RUN: | FileCheck %s
+// RUN: rm -rf %t
+// CHECK: running pass 0
+// CHECK: wrote{{.*}}per_pass_score
+// CHECK: wrote{{.*}}pass-summary
+// CHECK: wrote{{.*}}overall-pass-summary
+
+int main() {
+    return 0;
+}

cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/clang-opt-bisect/clang-opt-bisect.cpp

 // Purpose:
 //     Check the `clang-opt-bisect` tool runs with typical input.
 //
 // RUN: true
 // RUN: %dexter_base clang-opt-bisect \
 // RUN:     --debugger %dexter_regression_test_debugger \
 // RUN:     --builder %dexter_regression_test_builder \
 // RUN:     --cflags "%dexter_regression_test_cflags" \
 // RUN:     --ldflags "%dexter_regression_test_ldflags" \
 // RUN:     -- %s \
 // RUN: | FileCheck %s
 // CHECK: running pass 0
-// CHECK: wrote{{.*}}per_pass_score
-// CHECK: wrote{{.*}}pass-summary
-// CHECK: wrote{{.*}}overall-pass-summary
 
 int main() {
     return 0;
 }