[Lldb-commits] [PATCH] D112047: [lldb/test] Update TestScriptedProcess to use skinny corefiles

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/examples/python/scripted_process/stack_core_scripted_process.py:1
+import os,struct,signal
+
----------------
This should live next to the test. I don't see a point of shipping this to users. 


================
Comment at: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py:94
 
-    @skipIfDarwin
+    def create_stack_skinny_corefile(self):
+        self.build()
----------------
This function does more than creating a stack core file: it deletes the current target and replaces it with the stack core file one. I would split this up in two functions, and maybe pass in the temporary file so you can do 

```
with tempfile.NamedTemporaryFile() as file:
```

in the caller to make it clear what the scope of the temporary file is. Currently, I assume it gets garbage collected after this function exits? 


================
Comment at: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py:124
 
-        scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
-        self.runCmd("command script import " + os.path.join(self.getSourceDir(),
-                                                            *scripted_process_example_relpath))
+        os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
+        self.runCmd("command script import " + os.path.join(self.scripted_process_example_abspath,
----------------
We should really find a better way to do this. Can you remind me why we can't pass that in the structured data? Other than for you test, I assume we rarely want a process launch for a scripted process, so maybe the default should be inverted. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112047/new/

https://reviews.llvm.org/D112047