bpo-32206: Pdb can now run modules #4752

mariocj89 · 2017-12-07T20:25:09Z

This change allows pdb to run modules and not only script.

This is quite useful when working on "runnable" modules as there is no other way to run them at the moment.

Example running unittest as a module

$ ./python -m pdb -m unittest
> /mnt/d/linux/cpython/Lib/unittest/__main__.py(3)<module>()
-> import sys
(Pdb) c
----------------------------------------------------------------------
Ran 0 tests in 0.000s
OK
The program exited via sys.exit(). Exit status: False
> /mnt/d/linux/cpython/Lib/unittest/__main__.py(3)<module>()
-> import sys
(Pdb)

There is an alternative implementation using runpy._run_module_as_main by applying:
mariocj89@e00c7b2 (It still has some issues to fix)

https://bugs.python.org/issue32206

Since PEP 338 we can run python modules as a script via `python -m module_name` but there is no way to run pdb on those. The proposal is to add a new argument "-m" to the pdb module to allow users to run `python -m pdb -m my_module_name`.

The test is validating where pdb starts therefore the part that is failing (checking modulename "/" file) is not relevant on that test

ncoghlan

The basic approach looks reasonable to me (I'd missed that we could cheat and use private APIs to get around the fact that the public runpy API can't actually handle the pdb use case properly).

Just a few minor comments inline, plus a potentially larger request related to the structure of the test cases.

It would also be good to add a What's New entry (in the standard library updates section - existing entries should give a reasonable sense of the expected style).

ncoghlan · 2018-01-05T07:27:37Z

Doc/library/pdb.rst

@@ -61,6 +61,11 @@ useful than quitting the debugger upon program's exit.
   :file:`pdb.py` now accepts a ``-c`` option that executes commands as if given
   in a :file:`.pdbrc` file, see :ref:`debugger-commands`.

+.. versionadded:: 3.7
+   :file:`pdb.py` now accepts a ``-m`` option that execute modules similar to how
+   ``python3 -m`` does. The debugger will stop in the first line like with a script.


Slight rephrasing of the second sentence here: As with a script, the debugger will pause execution just before the first line of the module.

ncoghlan · 2018-01-05T07:29:00Z

Doc/library/pdb.rst

@@ -61,6 +61,11 @@ useful than quitting the debugger upon program's exit.
   :file:`pdb.py` now accepts a ``-c`` option that executes commands as if given
   in a :file:`.pdbrc` file, see :ref:`debugger-commands`.

+.. versionadded:: 3.7
+   :file:`pdb.py` now accepts a ``-m`` option that execute modules similar to how


Replace "how" with "the way"

ncoghlan · 2018-01-05T07:31:33Z

Lib/pdb.py

+        __main__.__dict__.update({
+            "__name__": "__main__",
+            "__file__": self.mainpyfile,
+#            "__package__": module_name,  # Not needed, will rely on __spec__.parent


Even though it's not always needed these days, runpy still promises to set __package__, so this should set it too.

ncoghlan · 2018-01-05T07:39:26Z

Lib/test/test_pdb.py

@@ -1123,9 +1130,136 @@ def tearDown(self):
        support.unlink(support.TESTFN)


+class PdbModuleTestCase(PdbBaseTestCase):
+    """Re-runs all tests used for a script but using a module"""


Our preferred structure for this kind of testing is to put the actual test methods in a base class that isn't a unittest.TestCase subclass (so we only write them once and structurally ensure that they're consistent across test scenarios), and then add unittest.TestCase as a mixin in the subclasses.

Having two copies of the test methods is significantly less desirable, as that makes it easier for the test cases to diverge over time.

There is actually not that much being shared between the two test cases other than the "run module".
I guess you were referring to something like this

When I was going through the work I realized there is not that many tests in that test case that made sense running with both module/script. Most "basic" unit tests are covered in the doctests. If you want I can try to rework the whole suite (moving the doctests?) and the do the mixing thing. otherwise I've added a commit that just adds the new helper function to run a module and left the tests in the same suite.

bedevere-bot · 2018-01-05T07:41:56Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

mariocj89 · 2018-01-05T22:42:26Z

I have made the requested changes; please review again

bedevere-bot · 2018-01-05T22:42:27Z

Thanks for making the requested changes!

@ncoghlan: please review the changes made to this pull request.

mariocj89 · 2018-01-05T22:43:18Z

Thanks a lot for the review. There is still a quesion on how to structure the tests, I cannot see a clear solution other than the one I did in d77dc87 as there is not that much shareable atm.

ncoghlan · 2018-01-06T07:54:37Z

Flattening the test cases back into a single class works for me - it was the three-class structure without any shared test cases that looked odd.

Thanks for the patch!

mariocj89 and others added 6 commits December 3, 2017 13:31

WIP: Run modules as script with pdb

f47d1b7

Since PEP 338 we can run python modules as a script via `python -m module_name` but there is no way to run pdb on those. The proposal is to add a new argument "-m" to the pdb module to allow users to run `python -m pdb -m my_module_name`.

Fix pdb starting in runpy

db73bf7

fix infinite loop

1d59520

Add news entry

57c4ea1

Fix running modules out of path

e8788cd

Add further tests

a0226bd

the-knights-who-say-ni added the CLA signed label Dec 7, 2017

bedevere-bot added the awaiting review label Dec 7, 2017

mariocj89 and others added 3 commits December 13, 2017 09:35

Remove duplicated test

2d38a6e

Make test pass on windows

15ff839

The test is validating where pdb starts therefore the part that is failing (checking modulename "/" file) is not relevant on that test

Add entry in Docs/Library/pdb

7a1494a

mariocj89 force-pushed the pdb_run_module branch from 5acd330 to 7a1494a Compare December 13, 2017 22:59

ncoghlan requested changes Jan 5, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Jan 5, 2018

mariocj89 added 3 commits January 5, 2018 19:15

Add entry in standard library docs section

8134c27

Rewording from Nick's review

6c4eed5

Restructure pdb tests

d77dc87

bedevere-bot added awaiting change review and removed awaiting changes labels Jan 5, 2018

ncoghlan merged commit 9f1e5f1 into python:master Jan 6, 2018

bedevere-bot removed the awaiting change review label Jan 6, 2018

mariocj89 deleted the pdb_run_module branch January 6, 2018 09:01

mariocj89 mentioned this pull request Jan 6, 2018

bpo-32206: Update pdb usage to include new module option #5111

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-32206: Pdb can now run modules #4752

bpo-32206: Pdb can now run modules #4752

mariocj89 commented Dec 7, 2017 •

edited

Loading

ncoghlan left a comment

ncoghlan Jan 5, 2018

ncoghlan Jan 5, 2018

ncoghlan Jan 5, 2018

ncoghlan Jan 5, 2018

mariocj89 Jan 5, 2018

bedevere-bot commented Jan 5, 2018

mariocj89 commented Jan 5, 2018

bedevere-bot commented Jan 5, 2018

mariocj89 commented Jan 5, 2018

ncoghlan commented Jan 6, 2018

bpo-32206: Pdb can now run modules #4752

bpo-32206: Pdb can now run modules #4752

Conversation

mariocj89 commented Dec 7, 2017 • edited Loading

ncoghlan left a comment

Choose a reason for hiding this comment

ncoghlan Jan 5, 2018

Choose a reason for hiding this comment

ncoghlan Jan 5, 2018

Choose a reason for hiding this comment

ncoghlan Jan 5, 2018

Choose a reason for hiding this comment

ncoghlan Jan 5, 2018

Choose a reason for hiding this comment

mariocj89 Jan 5, 2018

Choose a reason for hiding this comment

bedevere-bot commented Jan 5, 2018

mariocj89 commented Jan 5, 2018

bedevere-bot commented Jan 5, 2018

mariocj89 commented Jan 5, 2018

ncoghlan commented Jan 6, 2018

mariocj89 commented Dec 7, 2017 •

edited

Loading