labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.


================
Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:35
+
+        reproducer_patch = tempfile.NamedTemporaryFile().name
+
----------------
s/patch/path


================
Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:53-54
+        outs, errs = capture.communicate()
+        self.assertTrue('Process {} stopped'.format(pid) in outs)
+        self.assertTrue('Reproducer written' in outs)
+
----------------
self.assertIn(needle, haystack)


================
Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:66-75
+        replay = subprocess.Popen([
+            lldbtest_config.lldbExec, '-b', '-o',
+            'reproducer dump -f {} -p process'.format(reproducer_patch)
+        ],
+                                  stdin=subprocess.PIPE,
+                                  stdout=subprocess.PIPE,
+                                  stderr=subprocess.PIPE)
----------------
Would it be possible to run this in the context of the current process (via 
self.expect?)


================
Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:74-75
+        outs, errs = replay.communicate()
+        self.assertTrue('name = {}'.format(exe))
+        self.assertTrue('pid = {}'.format(pid))
+
----------------
I guess you meant assertIn here too


================
Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:77-82
+        # Remove the reproducer but don't complain in case the directory was
+        # never created.
+        try:
+            shutil.rmtree(reproducer_patch)
+        except OSError:
+            pass
----------------
The reproducer dir will still linger on if the test fails for any reason. If 
you just put this in the build directory (by (ab)using 
`self.getBuildArtifact`), then maybe you don't need to clean it up, as it will 
be automatically deleted the next time the test suite runs...


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

https://reviews.llvm.org/D75877



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to