Don't stop on logpoints with conditions. Fixes #1146

Author: fabioz

src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_cython.c

@@ -704,6 +704,7 @@ cdef class PyDBFrame:
         cdef bint has_exception_breakpoints;
         cdef bint can_skip;
         cdef bint stop;
+        cdef bint stop_on_plugin_breakpoint;
         cdef PyDBAdditionalThreadInfo info;
         cdef int step_cmd;
         cdef int line;
@@ -714,7 +715,6 @@ cdef class PyDBFrame:
         cdef dict breakpoints_for_file;
         cdef dict stop_info;
         cdef str curr_func_name;
-        cdef bint exist_result;
         cdef dict frame_skips_cache;
         cdef object frame_cache_key;
         cdef tuple line_cache_key;
@@ -1036,13 +1036,12 @@ cdef class PyDBFrame:
             # if DEBUG: print('NOT skipped: %s %s %s %s' % (frame.f_lineno, frame.f_code.co_name, event, frame.__class__.__name__))
 
             try:
-                flag = False
+                stop_on_plugin_breakpoint = False
                 # return is not taken into account for breakpoint hit because we'd have a double-hit in this case
                 # (one for the line and the other for the return).
 
                 stop_info = {}
                 breakpoint = None
-                exist_result = False
                 stop = False
                 stop_reason = 111
                 bp_type = None
@@ -1057,33 +1056,25 @@ cdef class PyDBFrame:
                     breakpoint = breakpoints_for_file[line]
                     new_frame = frame
                     stop = True
-                    if step_cmd in (108, 159) and (self._is_same_frame(stop_frame, frame) and is_line):
-                        stop = False  # we don't stop on breakpoint if we have to stop by step-over (it will be processed later)
+
                 elif plugin_manager is not None and main_debugger.has_plugin_line_breaks:
                     result = plugin_manager.get_breakpoint(main_debugger, self, frame, event, self._args)
                     if result:
-                        exist_result = True
-                        flag, breakpoint, new_frame, bp_type = result
+                        stop_on_plugin_breakpoint, breakpoint, new_frame, bp_type = result
 
                 if breakpoint:
                     # ok, hit breakpoint, now, we have to discover if it is a conditional breakpoint
                     # lets do the conditional stuff here
                     if breakpoint.expression is not None:
                         main_debugger.handle_breakpoint_expression(breakpoint, info, new_frame)
-                        if breakpoint.is_logpoint and info.pydev_message is not None and len(info.pydev_message) > 0:
-                            cmd = main_debugger.cmd_factory.make_io_message(info.pydev_message + os.linesep, '1')
-                            main_debugger.writer.add_command(cmd)
 
-                    if stop or exist_result:
+                    if stop or stop_on_plugin_breakpoint:
                         eval_result = False
                         if breakpoint.has_condition:
                             eval_result = main_debugger.handle_breakpoint_condition(info, breakpoint, new_frame)
-
-                        if breakpoint.has_condition:
                             if not eval_result:
                                 stop = False
-                        elif breakpoint.is_logpoint:
-                            stop = False
+                                stop_on_plugin_breakpoint = False
 
                     if is_call and (frame.f_code.co_name in ('<lambda>', '<module>') or (line == 1 and frame.f_code.co_name.startswith('<cell'))):
                         # If we find a call for a module, it means that the module is being imported/executed for the
@@ -1099,6 +1090,15 @@ cdef class PyDBFrame:
 
                         return self.trace_dispatch
 
+                    # Handle logpoint (on a logpoint we should never stop).
+                    if (stop or stop_on_plugin_breakpoint) and breakpoint.is_logpoint:
+                        stop = False
+                        stop_on_plugin_breakpoint = False
+
+                        if info.pydev_message is not None and len(info.pydev_message) > 0:
+                            cmd = main_debugger.cmd_factory.make_io_message(info.pydev_message + os.linesep, '1')
+                            main_debugger.writer.add_command(cmd)
+
                 if main_debugger.show_return_values:
                     if is_return and (
                             (info.pydev_step_cmd in (108, 159, 128) and (self._is_same_frame(stop_frame, frame.f_back))) or
@@ -1125,7 +1125,7 @@ cdef class PyDBFrame:
                         suspend_other_threads=breakpoint and breakpoint.suspend_policy == "ALL",
                     )
 
-                elif flag and plugin_manager is not None:
+                elif stop_on_plugin_breakpoint and plugin_manager is not None:
                     result = plugin_manager.suspend(main_debugger, thread, frame, bp_type)
                     if result:
                         frame = result

src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_cython.pyx

@@ -557,6 +557,7 @@ def _is_same_frame(self, target_frame, current_frame):
     #     cdef bint has_exception_breakpoints;
     #     cdef bint can_skip;
     #     cdef bint stop;
+    #     cdef bint stop_on_plugin_breakpoint;
     #     cdef PyDBAdditionalThreadInfo info;
     #     cdef int step_cmd;
     #     cdef int line;
@@ -567,7 +568,6 @@ def _is_same_frame(self, target_frame, current_frame):
     #     cdef dict breakpoints_for_file;
     #     cdef dict stop_info;
     #     cdef str curr_func_name;
-    #     cdef bint exist_result;
     #     cdef dict frame_skips_cache;
     #     cdef object frame_cache_key;
     #     cdef tuple line_cache_key;
@@ -889,13 +889,12 @@ def trace_dispatch(self, frame, event, arg):
             # if DEBUG: print('NOT skipped: %s %s %s %s' % (frame.f_lineno, frame.f_code.co_name, event, frame.__class__.__name__))
 
             try:
-                flag = False
+                stop_on_plugin_breakpoint = False
                 # return is not taken into account for breakpoint hit because we'd have a double-hit in this case
                 # (one for the line and the other for the return).
 
                 stop_info = {}
                 breakpoint = None
-                exist_result = False
                 stop = False
                 stop_reason = CMD_SET_BREAK
                 bp_type = None
@@ -910,33 +909,25 @@ def trace_dispatch(self, frame, event, arg):
                     breakpoint = breakpoints_for_file[line]
                     new_frame = frame
                     stop = True
-                    if step_cmd in (CMD_STEP_OVER, CMD_STEP_OVER_MY_CODE) and (self._is_same_frame(stop_frame, frame) and is_line):
-                        stop = False  # we don't stop on breakpoint if we have to stop by step-over (it will be processed later)
+
                 elif plugin_manager is not None and main_debugger.has_plugin_line_breaks:
                     result = plugin_manager.get_breakpoint(main_debugger, self, frame, event, self._args)
                     if result:
-                        exist_result = True
-                        flag, breakpoint, new_frame, bp_type = result
+                        stop_on_plugin_breakpoint, breakpoint, new_frame, bp_type = result
 
                 if breakpoint:
                     # ok, hit breakpoint, now, we have to discover if it is a conditional breakpoint
                     # lets do the conditional stuff here
                     if breakpoint.expression is not None:
                         main_debugger.handle_breakpoint_expression(breakpoint, info, new_frame)
-                        if breakpoint.is_logpoint and info.pydev_message is not None and len(info.pydev_message) > 0:
-                            cmd = main_debugger.cmd_factory.make_io_message(info.pydev_message + os.linesep, '1')
-                            main_debugger.writer.add_command(cmd)
 
-                    if stop or exist_result:
+                    if stop or stop_on_plugin_breakpoint:
                         eval_result = False
                         if breakpoint.has_condition:
                             eval_result = main_debugger.handle_breakpoint_condition(info, breakpoint, new_frame)
-
-                        if breakpoint.has_condition:
                             if not eval_result:
                                 stop = False
-                        elif breakpoint.is_logpoint:
-                            stop = False
+                                stop_on_plugin_breakpoint = False
 
                     if is_call and (frame.f_code.co_name in ('<lambda>', '<module>') or (line == 1 and frame.f_code.co_name.startswith('<cell'))):
                         # If we find a call for a module, it means that the module is being imported/executed for the
@@ -952,6 +943,15 @@ def trace_dispatch(self, frame, event, arg):
 
                         return self.trace_dispatch
 
+                    # Handle logpoint (on a logpoint we should never stop).
+                    if (stop or stop_on_plugin_breakpoint) and breakpoint.is_logpoint:
+                        stop = False
+                        stop_on_plugin_breakpoint = False
+
+                        if info.pydev_message is not None and len(info.pydev_message) > 0:
+                            cmd = main_debugger.cmd_factory.make_io_message(info.pydev_message + os.linesep, '1')
+                            main_debugger.writer.add_command(cmd)
+
                 if main_debugger.show_return_values:
                     if is_return and (
                             (info.pydev_step_cmd in (CMD_STEP_OVER, CMD_STEP_OVER_MY_CODE, CMD_SMART_STEP_INTO) and (self._is_same_frame(stop_frame, frame.f_back))) or
@@ -978,7 +978,7 @@ def trace_dispatch(self, frame, event, arg):
                         suspend_other_threads=breakpoint and breakpoint.suspend_policy == "ALL",
                     )
 
-                elif flag and plugin_manager is not None:
+                elif stop_on_plugin_breakpoint and plugin_manager is not None:
                     result = plugin_manager.suspend(main_debugger, thread, frame, bp_type)
                     if result:
                         frame = result

src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_frame.py

@@ -4342,13 +4342,13 @@ def test_frame_eval_mode_corner_case_03(case_setup):
             hit = writer.wait_for_breakpoint_hit(line=line + 1, reason=REASON_STEP_OVER)
 
             writer.write_step_over(hit.thread_id)  # i.e.: check that the jump target is still ok.
-            hit = writer.wait_for_breakpoint_hit(line=line, reason=REASON_STEP_OVER)
+            hit = writer.wait_for_breakpoint_hit(line=line, reason=REASON_STOP_ON_BREAKPOINT)
 
             writer.write_step_over(hit.thread_id)
             hit = writer.wait_for_breakpoint_hit(line=line + 1, reason=REASON_STEP_OVER)
 
             writer.write_step_over(hit.thread_id)
-            hit = writer.wait_for_breakpoint_hit(line=line, reason=REASON_STEP_OVER)
+            hit = writer.wait_for_breakpoint_hit(line=line, reason=REASON_STOP_ON_BREAKPOINT)
 
             writer.write_run_thread(hit.thread_id)
 

src/debugpy/_vendored/pydevd/tests_python/test_debugger.py

@@ -521,38 +521,53 @@ def write_get_source(self, source_reference, success=True):
         return response
 
 
-def test_case_json_logpoints(case_setup_dap):
+@pytest.mark.parametrize('scenario', ['basic', 'condition', 'hitCondition'])
+def test_case_json_logpoints(case_setup_dap, scenario):
     with case_setup_dap.test_file('_debugger_case_change_breaks.py') as writer:
         json_facade = JsonFacade(writer)
 
         json_facade.write_launch()
         break_2 = writer.get_line_index_with_content('break 2')
         break_3 = writer.get_line_index_with_content('break 3')
-        json_facade.write_set_breakpoints(
-            [break_2, break_3],
-            line_to_info={
-                break_2: {'log_message': 'var {repr("_a")} is {_a}'}
-        })
+        if scenario == 'basic':
+            json_facade.write_set_breakpoints(
+                [break_2, break_3],
+                line_to_info={
+                    break_2: {'log_message': 'var {repr("_a")} is {_a}'}
+            })
+        elif scenario == 'condition':
+            json_facade.write_set_breakpoints(
+                [break_2, break_3],
+                line_to_info={
+                    break_2: {'log_message': 'var {repr("_a")} is {_a}', 'condition': 'True'}
+            })
+        elif scenario == 'hitCondition':
+            json_facade.write_set_breakpoints(
+                [break_2, break_3],
+                line_to_info={
+                    break_2: {'log_message': 'var {repr("_a")} is {_a}', 'hit_condition': '1'}
+            })
         json_facade.write_make_initial_run()
 
         # Should only print, not stop on logpoints.
-        messages = []
-        while True:
-            output_event = json_facade.wait_for_json_message(OutputEvent)
+
+        # Just one hit at the end (break 3).
+        json_facade.wait_for_thread_stopped(line=break_3)
+        json_facade.write_continue()
+
+        def accept_message(output_event):
             msg = output_event.body.output
             ctx = output_event.body.category
 
             if ctx == 'stdout':
                 msg = msg.strip()
-                if msg == "var '_a' is 2":
-                    messages.append(msg)
+                return msg == "var '_a' is 2"
 
-                if len(messages) == 2:
-                    break
-
-        # Just one hit at the end (break 3).
-        json_facade.wait_for_thread_stopped(line=break_3)
-        json_facade.write_continue()
+        messages = json_facade.mark_messages(OutputEvent, accept_message)
+        if scenario == 'hitCondition':
+            assert len(messages) == 1
+        else:
+            assert len(messages) == 2
 
         writer.finished_ok = True
 

src/debugpy/_vendored/pydevd/tests_python/test_debugger_json.py

@@ -205,18 +205,6 @@ def code_to_debug():
                 },
             )
 
-        if condition:
-            session.wait_for_stop(
-                "breakpoint", expected_frames=[some.dap.frame(code_to_debug, line="bp")]
-            )
-
-            var_i = session.get_variable("i")
-            assert var_i == some.dict.containing(
-                {"name": "i", "evaluateName": "i", "type": "int", "value": "5"}
-            )
-
-            session.request_continue()
-
         session.wait_for_stop(
             "breakpoint",
             expected_frames=[some.dap.frame(code_to_debug, line="wait_for_output")],
@@ -228,9 +216,12 @@ def code_to_debug():
     if "internalConsole" not in str(run):
         assert not session.captured_stdout()
 
-    expected_stdout = "".join(
-        (r"{0}\r?\n".format(re.escape(str(i))) for i in range(0, 10))
-    )
+    if condition:
+        expected_stdout = "5\r?\n"
+    else:
+        expected_stdout = "".join(
+            (r"{0}\r?\n".format(re.escape(str(i))) for i in range(0, 10))
+        )
     expected_stderr = "".join(
         (r"{0}\r?\n".format(re.escape(str(i * 10))) for i in range(0, 10))
     )