[Lldb-commits] [PATCH] D42281: WIP: compile the LLDB tests out-of-tree

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

This looks good to me.  I think it would be cleaner if there were a getSourceFileSpec equivalent to getBuildArtifact.

I had a few inline trivial questions.

Also, before you forget all the things you had to do, you need to write down the new rules in the README.testsuite.  Might also be good to put a brief note in the sample_test testcases, since I imagine folks are just going to copy that and put it somewhere to make new tests, so reminding them there might not be a bad idea.



================
Comment at: packages/Python/lldbsuite/test/api/check_public_api_headers/TestPublicAPIHeaders.py:46
         self.line_to_break = line_number(
-            self.source, '// Set breakpoint here.')
+            self.getBuildArtifact(self.source), '// Set breakpoint here.')
 
----------------
Why do you have to call getBuildArtifact on the main source file?


================
Comment at: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/TestBreakpointSetRestart.py:35-37
             self.BREAKPOINT_TEXT, lldb.SBFileSpec(
                 os.path.join(
+                    self.getSourceDir(), 'main.cpp')))
----------------
This is beginning to show up a bunch.  Maybe TestBase should have:

  def getSourceFileSpec( filename):
    fileSpec = lldb.SBFileSpec()
    fileSpec.SetDirectory(self.getSourceDir())
    fileSpec.SetFilename(filename)
    return fileSpec


================
Comment at: packages/Python/lldbsuite/test/functionalities/exec/TestExec.py:47-48
         if self.getArchitecture() == 'x86_64':
-            source = os.path.join(os.getcwd(), "main.cpp")
-            o_file = os.path.join(os.getcwd(), "main.o")
+            source = os.path.join(self.getSourceDir(), "main.cpp")
+            o_file = self.getBuildArtifact("main.o")
             execute_command(
----------------
Maybe also a TestBase getSourcePath(filename) that returns a string?  That might be easier to read.


================
Comment at: packages/Python/lldbsuite/test/plugins/builder_base.py:224-232
+    #  #import traceback
+    #  # traceback.print_stack()
+    #  commands = []
+    #  if os.path.isfile("Makefile"):
+    #      commands.append(getMake("") + ["clean", getCmdLine(dictionary)])
+    #   
+    #  runBuildCommands(commands, sender=sender)
----------------
Probably can delete this now...


================
Comment at: packages/Python/lldbsuite/test/types/AbstractBase.py:37-38
         self.exe_name = self.testMethodName
-        self.golden_filename = os.path.join(os.getcwd(), "golden-output.txt")
+        self.golden_filename = os.path.join(self.getBuildDir(),
+                                            "golden-output.txt")
 
----------------
Why isn't this getBuildArtifact("golden-output.txt")?


https://reviews.llvm.org/D42281