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