[LLDB] Fix sync issue in TestVSCode_launch.test_progress_events

[omjavaid]

Summary

This fixes flakiness in TestVSCode_launch.test_progress_events
vscode.progress_events some times failed to populate in time for the for the follow up iteration.

Adding a minor delay before the the for the loop fixes the issue.

[clayborg]

lgtm!

[labath]

Isn't there a better way to ensure synchronization here? Maybe, executing some command (setting a breakpoint, for instance), that will ensure that all symbols have been parsed?

[clayborg]

In D99497#2656502, @labath wrote:

Isn't there a better way to ensure synchronization here? Maybe, executing some command (setting a breakpoint, for instance), that will ensure that all symbols have been parsed?

That is what I attempted do in the test: set a breakpoint at 'main' which should trigger all of the needed events. Then we run to a breakpoint and hit it. and after we hit this breakpoint, then we check the progress events.

One thing we could do to figure things out would be to dump the vscode.txt file for this test into the failing test. This would allow us to see what the packets looked like when a test fails. Is there a cleanup function or something we can do in the VS code tests that allows a test case to know it has an error and add some stuff to the test suite output??

[labath]

In D99497#2656600, @clayborg wrote:

In D99497#2656502, @labath wrote:

Isn't there a better way to ensure synchronization here? Maybe, executing some command (setting a breakpoint, for instance), that will ensure that all symbols have been parsed?

That is what I attempted do in the test: set a breakpoint at 'main' which should trigger all of the needed events. Then we run to a breakpoint and hit it. and after we hit this breakpoint, then we check the progress events.

Ok, I see what you mean. I looked (briefly) at the code, and my impression is that this dance will ensure that the progress events get /sent/, but they will not ensure that they are actually receieved/processed (as the receiving end is completely asynchronous).

The next question then becomes: is that intentional? If it is, then a delay is indeedall we can do (and I suspect we might need even more than 1s to sufficiently safe). Having progress events come after the operation that uses their results completes would be a bit weird, though it probably won't cause much problems in practice.

This situation actually reminds me of what we do with inferior stdout. That also used to be fully asynchronous (on several levels), but that ended up causing problems (for tests, at least), as we would try to assert that the inferior has written something after it hits a breakpoint, but that text would still be in flight. We solved that by explicitly flushing any pending output before sending the "stopped" event. One could imagine doing something similar in vscode as well -- before replying to a command (in this case, breakpoint set), check the progress listener, and flush any pending events.

One thing we could do to figure things out would be to dump the vscode.txt file for this test into the failing test. This would allow us to see what the packets looked like when a test fails. Is there a cleanup function or something we can do in the VS code tests that allows a test case to know it has an error and add some stuff to the test suite output??

There are several ways to do that. The simplest (and most useful, I think) would be to just stream the messages (both sent and received) to stdout if self.traceOn() is true. I don't think it will help much in this case, but it could be pretty useful in the future.

[omjavaid]

I am going ahead and commit this change till we find a better way to fix the sync issue. sleep time set to 2s now.

[labath]

The test is still flaky (http://lab.llvm.org:8011/#/builders/68/builds/10124, http://lab.llvm.org:8011/#/builders/68/builds/10118, http://lab.llvm.org:8011/#/builders/68/builds/10113, ...) In fact, I think it has gotten flakier since the timeouts were added... I'm disabling it.

lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

def test_terminate_commands(self):
         # "terminateCommands"
         self.vscode.request_disconnect(terminateDebuggee=True)
         output = self.collect_console(duration=1.0)
         self.verify_commands('terminateCommands', output, terminateCommands)
 
 
     @skipIfWindows
     @skipIfRemote
-    @skipIf(oslist=["linux"])
     def test_progress_events(self):
         '''
             Tests the progress events to ensure we are receiving them.
         '''
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
         # Set a breakpoint at 'main'. This will cause all of the symbol tables
         # for all modules in LLDB to be parsed and we should get a progress
def test_progress_events(self):
         symtab_progress_shlibs = set()
         # Get a list of modules in the current target so we can verify that
         # we do in fact get a progress event for each shared library.
         target_shlibs = self.vscode.get_modules()
 
         # Iterate over all progress events and save all start and end IDs, and
         # remember any shared libraries that got symbol table parsing progress
         # events.
+        # Sleep for 2 seconds to make sure progress_events gets populated
+        time.sleep(2)
         for progress_event in self.vscode.progress_events:
             event_type = progress_event['event']
             if event_type == 'progressStart':
                 progressStart_ids.add(progress_event['body']['progressId'])
                 title = progress_event['body']['title']
                 if title.startswith('Parsing symbol table for '):
                     symtab_progress_shlibs.add(title[25:])
             if event_type == 'progressEnd':