[Lldb-commits] [PATCH] D61687: Update Python tests for lldb-server on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py:205-217
     @skipUnlessPlatform(["linux"])
     @llgs_test
     def test_qProcessInfo_does_not_contain_cputype_cpusubtype_llgs_linux(self):
         self.init_llgs_test()
         self.build()
         self.qProcessInfo_does_not_contain_keys(set(['cputype', 'cpusubtype']))
+
----------------
These two tests can also be merged and made `@skipIfDarwin` as that is the only platform using the cpu(sub)type fields.


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:208-212
+    # In current implementation of llgs on Windows, as a response to '\x03' packet, the debugger
+    # of the native process will trigger a call to DebugBreakProcess that will create a new thread
+    # to handle the exception debug event. So one more stop thread will be notified to the
+    # delegate, e.g. llgs.  So tests below to assert the stop threads number will all fail.
+    @expectedFailureAll(oslist=["windows"])
----------------
labath wrote:
> Is this something that we consider to be a bug, or is it just how debugging is supposed to work on windows? If it's a bug then fine, but if not then we might consider adjusting the expectations in the test (or just skipping it).
You marked this as done, but it's not obvious how is this comment resolved (or indeed, if it needs to be resolved). Can you elaborate?


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py:146-153
         # All but one thread should report no stop reason.
-        self.assertEqual(no_stop_reason_count, thread_count - 1)
+        triple = self.dbg.GetSelectedPlatform().GetTriple()
+
+        # Consider one more thread created by calling DebugBreakProcess.
+        if re.match(".*-.*-windows", triple):
+            self.assertGreaterEqual(no_stop_reason_count, thread_count - 1)
+        else:
----------------
labath wrote:
> I think this assertion should just be deleted. If the assertion below (that one thread has a stop reason) is true, then it trivially true that the rest of the threads don't have one. Whether or not a platforms spawns an extra thread does not seem to be relevant for this test.
This comment doesn't appear to be "done".


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/inferior-crash/TestGdbRemoteAbort.py:40
 
+    @skipIfWindows # For now the signo in T* packet is always 0.
     @llgs_test
----------------
labath wrote:
> Do you have any plans for changing this? Given that windows does not support signals, maybe it should stay 0...
I'd like to hear your thoughts on this...

If the idea is for the signal number to stay zero, then the comment shouldn't say "for now" but instead say something to the effect that abort() does not cause a signal to be raised on windows because windows doesn't have signals. If the idea is to change this later, then I'd like to understand how/why.


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

https://reviews.llvm.org/D61687