GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-29598 Add couple of unit tests for pdb module by archy-rock3t-cloud · Pull Request #218 · python/cpython

bpo-29598 Add couple of unit tests for pdb module #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
archy-rock3t-cloud wants to merge 1 commit into python:master from archy-rock3t-cloud:bpo-29598-add-unit-tests-for-pdb

Uh oh!

There was an error while loading. Please reload this page.

Closed

bpo-29598 Add couple of unit tests for pdb module #218

Changes from all commits
Commits
Show all changes
1 commit
Select commit
fcf80b0
bpo-29598 Add couple of unit tests for pdb module
archy-rock3t-cloud Feb 21, 2017
File filter
Filter by extension

Clear filters
Conversations
Failed to load comments.
Loading

Uh oh!

There was an error while loading. Please reload this page.

Jump to
Jump to file
Failed to load files.
Loading

Uh oh!

There was an error while loading. Please reload this page.

Diff view
Diff view

There are no files selected for viewing

40 changes: 40 additions & 0 deletions 40 Lib/test/test_pdb.py
Show comments
View file
Open in desktop
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 Down Expand 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()):
Comment thread

@methane methane Feb 28, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@archy-rock3t-cloud archy-rock3t-cloud Feb 28, 2017

Copy link
Copy Markdown
Author

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@methane methane Feb 28, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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 of test target, not test runner.

What is module_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 frame in this test, not capturing from caller of this test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@methane methane Feb 28, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@archy-rock3t-cloud archy-rock3t-cloud Feb 28, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Author

There was a problem hiding this comment.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@methane methane Feb 28, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@archy-rock3t-cloud archy-rock3t-cloud Feb 28, 2017

Copy link
Copy Markdown
Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@methane methane Feb 28, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@archy-rock3t-cloud archy-rock3t-cloud Feb 28, 2017

Copy link
Copy Markdown
Author

There was a problem hiding this comment.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
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)
Comment thread Outdated

@methane methane Feb 22, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@xdegaye xdegaye Feb 24, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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 the normative 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()).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@methane methane Feb 25, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@xdegaye xdegaye Feb 26, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@xdegaye xdegaye Feb 26, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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 ?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@methane methane Feb 28, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@methane methane Feb 28, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

Expand Down
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.