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

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 10 10:40:25 PST 2017


MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

LGTM, nitpicks below



================
Comment at: lnt/tests/test_suite.py:530-536
+    def _get_path(self):
         path = self._base_path
 
         if not os.path.exists(path):
             mkdir_p(path)
 
+        return path
----------------
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.



================
Comment at: lnt/tests/test_suite.py:533-534
 
         if not os.path.exists(path):
             mkdir_p(path)
 
----------------
I consider `if not exists: create` an anti-pattern, just do `mkdir_p()` it'll be fine if the directory already exists.


================
Comment at: lnt/tests/test_suite.py:538
 
-        if not self.configured and self._need_to_configure(path):
+    def configure_if_needed(self):
+        path = self._get_path()
----------------
Could this be `_configure_if_neede()`?


https://reviews.llvm.org/D29821





More information about the llvm-commits mailing list