[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[daniel hahler]

daniel hahler  added the comment:

Just for reference: I've just spent quite some time debugging missing coverage 
with pytest, and it turned out that part of it is because of this issue.  Most 
of the pdb related tests run in a subprocess (via pexpect), but not all of them.

I am using a wrapper now to restore this manually 
(https://github.com/pytest-dev/pytest/pull/5406).

Even though I was aware of this issue, it took me quite a while, mainly because 
this started out with slash related issues for Windows only, and I was 
debugging this on/via CI only at first.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[Xavier de Gaye]

Xavier de Gaye  added the comment:

The main goal of test coverage is to verify that the tests cover the code that 
is being tested, it is not to get '100 %' printed. Measuring the coverage of 
the tests themselves is entirely another matter and not very useful. Since it 
is not possible to measure the coverage of code executed by a trace function, 
attempting to increase the measure of the coverage of the tests themselves that 
test pdb, just for the sake of it, does not justify the added complexity and 
possible source of confusion.

-1 for this change.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[daniel hahler]

daniel hahler  added the comment:

>From my initial comment:

> I've done this via monkeypatching for pdbpp's tests, which resulted in an 
> easy 2% coverage gain (https://github.com/antocuni/pdb/pull/253).

It will also affect running tests with coverage (in general, or using tools 
like pytest-testmon), and using pdb.set_trace then (either for debugging, or 
via post-mortem, e.g. pytest's `--pdb`).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[Xavier de Gaye]

Xavier de Gaye  added the comment:

> Is getting accurate coverage reports not enough?

But it does not achieve that goal, does it ?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[Rémi Lapeyre]

Rémi Lapeyre  added the comment:

> The proposed change is not backward compatible.

Indeed, I missed that thanks. I think I could fix that.

> A motivation for this change should be provided first

Is getting accurate coverage reports not enough?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[Xavier de Gaye]

Xavier de Gaye  added the comment:

> Would it work to store it on the class then (once)?

A motivation for this change should be provided first: the incorrect behavior 
it is trying to fix or the enhancement it is providing.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[daniel hahler]

daniel hahler  added the comment:

I've just found that I've created an issue with regard to `do_debug` for this 
already (https://bugs.python.org/issue36388), and a PR: 
https://github.com/python/cpython/pull/12479.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[daniel hahler]

daniel hahler  added the comment:

> In that case the proposed change builds an implicit stack of trace functions 
> that is increased each time the interpreter executes this hard breakpoint

Very valid and good point.

Would it work to store it on the class then (once)?

FWIW: pdbpp uses a global Pdb instance - mainly to support the use case of hard 
breakpoints like this (to remember/re-use its configuration then).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[Xavier de Gaye]

Xavier de Gaye  added the comment:

> While typically pdb is not used in tests, it is just good practice, given 
> that there can only be a single trace function.

IMO invoking "good practice" is not sufficient to motivate such a change.

The proposed change is not backward compatible. A common use case for pdb is to 
insert a so-called hard breakpoint (import pdb; pdb.set_trace()) line in 
existing code. In that case the proposed change builds an implicit stack of 
trace functions that is increased each time the interpreter executes this hard 
breakpoint (when set in a loop or in a function invoked multiple times for 
example). When the user hits the 'continue' pdb command, then pdb stops at the 
next function call if the hard breakpoint has been hit more than once instead 
of leaving the pdb session for good as it does currently.

--
nosy: +xdegaye

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[daniel hahler]

daniel hahler  added the comment:

Looks great, thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[Rémi Lapeyre]

Rémi Lapeyre  added the comment:

Hi @blueyed, can you confirm PR 13497 solve your issue ?

--
nosy: +remi.lapeyre

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[Rémi Lapeyre]

Change by Rémi Lapeyre :


--
keywords: +patch
pull_requests: +13413
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[daniel hahler]

Change by daniel hahler :


--
versions: +Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37011] pdb: restore original tracing function instead of sys.settrace(None)

[daniel hahler]

New submission from daniel hahler :

bdb/pdb currently uses `sys.settrace(None)` when uninstalling its trace 
function (trace_dispatch), but should rather store the original trace function 
in the beginning and use this instead of `None`.

While typically pdb is not used in tests, it is just good practice, given that 
there can only be a single trace function.

I've done this via monkeypatching for pdbpp's tests, which resulted in an easy 
2% coverage gain (https://github.com/antocuni/pdb/pull/253).

--
components: Library (Lib)
messages: 343188
nosy: blueyed
priority: normal
severity: normal
status: open
title: pdb: restore original tracing function instead of sys.settrace(None)
type: behavior
versions: Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com