Feb 21, 2017 · Feb 28, 2017 · Feb 28, 2017 · Feb 28, 2017 · Feb 28, 2017 · Feb 28, 2017
diff --git a/Lib/test/test_pdb.py b/Lib/test/test_pdb.py
 # A test suite for pdb; not very comprehensive at the moment.

 import dis
 import doctest
 import inspect
 import os
 import pdb
 import sys
            if save_home is not None:
                os.environ['HOME'] = save_home

    def test_find_function_with_invalid_filename(self):
        self.assertIsNone(pdb.find_function('foo', 'invalid filename'))

    def test_getsourcelines(self):
        def foo():
            pass  # pragma: no cover
        lines, lineno = inspect.findsource(foo)
        expected_lines = inspect.getblock(lines[lineno:])
        expected_lineno = lineno + 1
        actual_lines, actual_lineno = pdb.getsourcelines(foo)
        self.assertEqual(actual_lines, expected_lines)
        self.assertEqual(actual_lineno, expected_lineno)

    def test_getsourcelines_with_module_frame_obj(self):
        for frame, *unused in inspect.getouterframes(inspect.currentframe()):
            if frame.f_globals is frame.f_locals:
                module_frame = frame
                break
        expected_lines = inspect.findsource(module_frame)[0]
        lines, lineno = pdb.getsourcelines(module_frame)
        self.assertEqual(lines, expected_lines)
        self.assertEqual(lineno, 1)

    def test_getsourcelines_with_module_obj(self):
        module = inspect.getmodule(inspect.currentframe())
        expected_lines = inspect.findsource(module)[0]
        lines, lineno = pdb.getsourcelines(module)
        self.assertEqual(lines, expected_lines)
        self.assertEqual(lineno, 1)

    def test_lasti2lineno(self):
        code_obj = inspect.currentframe().f_code
        max_offset, max_lineno = max(dis.findlinestarts(code_obj))
        min_offset, min_lineno = min(dis.findlinestarts(code_obj))
        self.assertEqual(pdb.lasti2lineno(code_obj, max_offset), max_lineno)
        self.assertEqual(pdb.lasti2lineno(code_obj, max_offset+1), max_lineno)
        self.assertEqual(pdb.lasti2lineno(code_obj, min_offset-1), 0)

    def tearDown(self):
        support.unlink(support.TESTFN)
Original file line number	Diff line number	Diff line change
		@@ -1,6 +1,8 @@
		# A test suite for pdb; not very comprehensive at the moment.

		import dis
		import doctest
		import inspect
		import os
		import pdb
		import sys
Expand DownExpand Up		@@ -1110,6 +1112,44 @@ def test_readrc_kwarg(self):
		if save_home is not None:
		os.environ['HOME'] = save_home

		def test_find_function_with_invalid_filename(self):
		self.assertIsNone(pdb.find_function('foo', 'invalid filename'))

		def test_getsourcelines(self):
		def foo():
		pass # pragma: no cover
		lines, lineno = inspect.findsource(foo)
		expected_lines = inspect.getblock(lines[lineno:])
		expected_lineno = lineno + 1
		actual_lines, actual_lineno = pdb.getsourcelines(foo)
		self.assertEqual(actual_lines, expected_lines)
		self.assertEqual(actual_lineno, expected_lineno)

		def test_getsourcelines_with_module_frame_obj(self):
		for frame, *unused in inspect.getouterframes(inspect.currentframe()):
Copy link Member methaneFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I don't like test relying on outer frame. I like test which doesn't relying test runner implementation. Copy link Author archy-rock3t-cloudFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. But documentation says that whitebox tests are preferred Copy link Member methaneFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. "whitebox test" is relying on implementation oftest target, nottest runner. What ismodule_frame here? Isn't it relying on test runner? It can be changed even if pdb and test_pdb is not changed. So I prefer creating module framein this test, not capturing from caller of this test. Copy link Member methaneFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. BTW, is there any difference between module frame and other frames from pdb's point of view? If there is no difference, function frame is far easier to create, maybe. Copy link Author archy-rock3t-cloudFeb 28, 2017• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. In this case I need specifically module frame(as you can see from pdb code itself) and it seems like it doesn't matter which module frame it will be. Can you please show how to create it manually? Copy link Member methaneFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. `frame = inspect.currentframe()` frame must be current test's frame. caller's frame shouldn't affects this test. Copy link Author archy-rock3t-cloudFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. It won't be module frame. Copy link Member methaneFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Ah, I'm sorry. I misread "I need" as "I don't need". Now I read pdb.getsourcelines(). If I'm correct, do you want to test this one line:`return lines, 1`? I don't know way to create module frame which is simple enough to test such simple one line. Copy link Author archy-rock3t-cloudFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Yes, I would like it to be tested. Initially, when I've seen test coverage of stdlib I wanted to write some tests to improve it. Now it seems like tough task, because it is not even clear who should approve my PRs. If I address all comments here and in bpo, whom can I ask to approve my PR?
		if frame.f_globals is frame.f_locals:
		module_frame = frame
		break
		expected_lines = inspect.findsource(module_frame)[0]
		lines, lineno = pdb.getsourcelines(module_frame)
		self.assertEqual(lines, expected_lines)
		self.assertEqual(lineno, 1)

		def test_getsourcelines_with_module_obj(self):
		module = inspect.getmodule(inspect.currentframe())
		expected_lines = inspect.findsource(module)[0]
		lines, lineno = pdb.getsourcelines(module)
		self.assertEqual(lines, expected_lines)
		self.assertEqual(lineno, 1)

		def test_lasti2lineno(self):
		code_obj = inspect.currentframe().f_code
		max_offset, max_lineno = max(dis.findlinestarts(code_obj))
		min_offset, min_lineno = min(dis.findlinestarts(code_obj))
		self.assertEqual(pdb.lasti2lineno(code_obj, max_offset), max_lineno)
		self.assertEqual(pdb.lasti2lineno(code_obj, max_offset+1), max_lineno)
		self.assertEqual(pdb.lasti2lineno(code_obj, min_offset-1), 0)
Copy link Member methaneFeb 22, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I'm afraid this test is relying implementation detail. Doesn't#46 affects this? Is this test passes on other Python implementations like PyPy or micropython? Copy link Contributor xdegayeFeb 24, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. What implementation detail ? The Python implementations like PyPy follow also thenormative description of co_lnotab. I may be wrong, but differences in the definitions of the structure of the AST between those implementations should not bear any impact on the definition of the structure of co_lnotab. As a side note, offsets increase monotonically so min_offset is the first offset yielded by findlinestarts() and max_offset is the last one (no need to invoke min() or max()). Copy link Member methaneFeb 25, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I didn't research deeply, I just worried. So if PyPy passes this, it's OK. I think attributes of Python's code object is implementation detail, even though PyPy follows. But it's not big problem until there are some Python implementation having different code implementation. Copy link Contributor xdegayeFeb 26, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I have started a discussion on python-dev about attributes of Python code objects being implementation detail. Copy link Contributor xdegayeFeb 26, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. @methane Assuming now that the specification of co_lnotab is an implementation detail, then the pdb and inspect modules from the CPython standard library may fail to run on a new Python implementation that does not follow the CPython co_lnotab specification. So why do you want to constrain test_pdb to run on this Python implementation when the pdb and inspect modules themselves would not ? Copy link Member methaneFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. @xdegaye I didn't say "I want to do ...". I was asked to review, and leave comment what I feel. I didn't say LGTM or "change required". Copy link Member methaneFeb 28, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Please ignore me when my comment doesn't make sense for you. I'm not familiar with this area.

		def tearDown(self):
		support.unlink(support.TESTFN)

Expand Down