[LNT] Run configure earlier in runtest test-suite [NFC]

[kristof.beyls]

Summary

This is in preparation for follow-on work where we'll extract the
compiler used from the CMakeCache rather than from the --cc
command line option. For that to work, however, the configure step
has to be run before the first time self._get_cc_info() gets called.

This also simplifies the need-to-configure logic a bit.

[MatzeB]

LGTM, nitpicks below

lnt/tests/test_suite.py

530–536 ↗

(On Diff #87983)

The _get_path() function boils down to to calling mkdir_p(self._base_path) should be easier/more obvious to just inline it instead of creating a function.

533–534 ↗

(On Diff #87983)

I consider if not exists: create an anti-pattern, just do mkdir_p() it'll be fine if the directory already exists.

538 ↗

(On Diff #87983)

Could this be _configure_if_neede()?

lnt/trunk/lnt/tests/test_suite.py

def run_test(self, name, args):
         if self.opts.timestamp_build:
             ts = self.start_time.replace(' ', '_').replace(':', '-')
             build_dir_name = "test-%s" % ts
         else:
             build_dir_name = "build"
         basedir = os.path.join(self.opts.sandbox_path, build_dir_name)
         self._base_path = basedir
 
+        cmakecache = os.path.join(self._base_path, 'CMakeCache.txt')
+        self.configured = not self.opts.run_configure and \
+            os.path.exists(cmakecache)
+
+        #  If we are doing diagnostics, skip the usual run and do them now.
+        if opts.diagnose:
+            return self.diagnose()
+
+        # configure, so we can extract toolchain information from the cmake
+        # output.
+        self._configure_if_needed()
+
         # We don't support compiling without testing as we can't get compile-
         # time numbers from LIT without running the tests.
         if opts.compile_multisample > opts.exec_multisample:
             note("Increasing number of execution samples to %d" %
                  opts.compile_multisample)
             opts.exec_multisample = opts.compile_multisample
 
         if opts.auto_name:
             # Construct the nickname from a few key parameters.
             cc_info = self._get_cc_info()
             cc_nick = '%s_%s' % (cc_info['cc_name'], cc_info['cc_build'])
             self.nick += "__%s__%s" % (cc_nick,
                                        cc_info['cc_target'].split('-')[0])
         note('Using nickname: %r' % self.nick)
 
         #  When we can't detect the clang version we use 0 instead. That
         # is a horrible failure mode because all of our data ends up going
         # to order 0.  The user needs to give an order if we can't detect!
         if opts.run_order is None:
             cc_info = self._get_cc_info()
             if cc_info['inferred_run_order'] == 0:
                 fatal("Cannot detect compiler version. Specify --run-order"
                       " to manually define it.")
 
-        #  If we are doing diagnostics, skip the usual run and do them now.
-        if opts.diagnose:
-            return self.diagnose()
         # Now do the actual run.
         reports = []
         json_reports = []
         for i in range(max(opts.exec_multisample, opts.compile_multisample)):
             c = i < opts.compile_multisample
             e = i < opts.exec_multisample
             run_report, json_data = self.run(compile=c, test=e)
             reports.append(run_report)
def run_test(self, name, args):
         csv_report_path = os.path.join(self._base_path,
                                        'test-results.csv')
         str_template = _lit_json_to_csv(json_reports)
         with open(csv_report_path, 'w') as fd:
             fd.write(str_template)
 
         return self.submit(report_path, self.opts, commit=True)
 
-    def run(self, compile=True, test=True):
-        path = self._base_path
+    def _configure_if_needed(self):
+        mkdir_p(self._base_path)
+        if not self.configured:
+            self._configure(self._base_path)
+            self._clean(self._base_path)
+            self.configured = True
 
-        if not os.path.exists(path):
-            mkdir_p(path)
+    def run(self, compile=True, test=True):
+        mkdir_p(self._base_path)
 
         if self.opts.pgo:
-            self._collect_pgo(path)
+            self._collect_pgo(self._base_path)
             self.trained = True
+            self.configured = False
 
-        if not self.configured and self._need_to_configure(path):
-            self._configure(path)
-            self._clean(path)
-            self.configured = True
+        self._configure_if_needed()
 
         if self.compiled and compile:
-            self._clean(path)
+            self._clean(self._base_path)
         if not self.compiled or compile:
-            self._make(path)
+            self._make(self._base_path)
             self.compiled = True
 
-        data = self._lit(path, test)
-        return self._parse_lit_output(path, data), data
+        data = self._lit(self._base_path, test)
+        return self._parse_lit_output(self._base_path, data), data
 
     def _create_merged_report(self, reports):
         if len(reports) == 1:
             return reports[0]
 
         machine = reports[0].machine
         run = reports[0].run
         run.end_time = reports[-1].run.end_time
         test_samples = sum([r.tests for r in reports], [])
         return lnt.testing.Report(machine, run, test_samples)
 
-    def _need_to_configure(self, path):
-        cmakecache = os.path.join(path, 'CMakeCache.txt')
-        return self.opts.run_configure or not os.path.exists(cmakecache)
-
     def _test_suite_dir(self):
         return self.opts.test_suite_root
 
     def _build_threads(self):
         return self.opts.build_threads or self.opts.threads
 
     def _test_threads(self):
         return self.opts.threads
def _get_lnt_code(self, code):
                 }[code]
 
     def _test_failed_to_compile(self, raw_name, path):
         # FIXME: Do we need to add ".exe" in windows?
         name = raw_name.rsplit('.test', 1)[0]
         return not os.path.exists(os.path.join(path, name))
 
     def _get_target_flags(self):
+        assert self.configured is True
         return shlex.split(self.opts.cppflags + self.opts.cflags)
 
     def _get_cc_info(self):
+        assert self.configured is True
         return lnt.testing.util.compilers.get_cc_info(self.opts.cc,
                                                       self._get_target_flags())
 
     def _parse_lit_output(self, path, data, only_test=False):
         LIT_METRIC_TO_LNT = {
             'compile_time': 'compile',
             'exec_time': 'exec',
             'score': 'score',
def diagnose(self):
         report_path = os.path.abspath(report_name)
 
         # Overwrite the report.
         if os.path.exists(report_path):
             shutil.rmtree(report_path)
         os.mkdir(report_path)
 
         path = self._base_path
-        if not os.path.exists(path):
-            mkdir_p(path)
         os.chdir(path)
 
         # Run with -save-temps
         cmd = self._configure(path, execute=False)
         cmd_temps = cmd + ['-DTEST_SUITE_DIAGNOSE_FLAGS=-save-temps']
 
         note(' '.join(cmd_temps))