twisted / twisted

Event-driven networking engine written in Python.
https://www.twisted.org
Other
5.48k stars 1.15k forks source link

TestCase.flushWarnings shouldn't use source files to implement its "offendingFunctions" feature #3598

Closed twisted-trac closed 15 years ago

twisted-trac commented 15 years ago
exarkun's avatar @exarkun reported
Trac ID trac#3598
Type defect
Created 2009-01-04 00:37:52Z
Branch https://github.com/twisted/twisted/tree/findlinestarts-3598

Using source files to determine which function a warning was emitted from is unreliable. It fails if:

  1. The source file is missing
  2. The source file is out of date with the bytecode file
  3. It's kind of slow

dis.findlinestarts can provide the necessary information and is now public.

Searchable metadata ``` trac-id__3598 3598 type__defect defect reporter__exarkun exarkun priority__high high milestone__ branch__branches_findlinestarts_3598 branches/findlinestarts-3598 branch_author__exarkun exarkun status__closed closed resolution__fixed fixed component__trial trial keywords__ time__1231029472000000 1231029472000000 changetime__1232114755000000 1232114755000000 version__None None owner__ ```
twisted-trac commented 15 years ago
exarkun's avatar @exarkun removed owner

Think this is in good shape now.

twisted-trac commented 15 years ago
radix's avatar @radix set owner to @exarkun

[1] Need to mention copyright of that code copied from Python

[2] in test_renamedSource:

You don't actually need to drop it so you can import it again, since it's going to be imported with the different name, right? Of course, you do need to clean it up, but you can do that at the end.

[3] Please change that function to not use single-letter variables that are used across more than one line.

[4] I don't know whether the following expression has off-by-one errors:

Can you please add tests for all of the edge cases, thank you.

twisted-trac commented 15 years ago
exarkun's avatar @exarkun removed owner

All points addressed, I think. Build results: http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/findlinestarts-3598

twisted-trac commented 15 years ago
mwhudson's avatar @mwhudson set owner to @exarkun

I think it looks fine. I have two small comments.

  1. Saying "inspect.getabsfile(aFunction) returns bad results sometimes." is a bit useless. What are bad results? When does it generate them?

  2. I know you apologise for it, but 'renamedsourcefile.py' is never actually renamed. I think something plain 'module.py' would be clearer.

These are minor enough that I don't want to review this again :) Please fix and land.

twisted-trac commented 13 years ago
Automation's avatar Automation removed owner
twisted-trac commented 15 years ago
exarkun's avatar @exarkun set status to closed

(In [26043]) Merge findlinestarts-3598

Author: exarkun Reviewer: radix, mwhudson Fixes: #3598

Change the implementation of twisted.trial.unittest.TestCase.flushWarnings so that it works properly when a warning is emitted from a module which is defined by a source file which has been renamed since it was compiled. Previously, the rename would cause file names to disagree in certain places and prevent warnings emitted from code in a renamed file from being flushed.

twisted-trac commented 15 years ago
exarkun's avatar @exarkun commented

(In [25848]) add/adjust comments in new test method to be more descriptive/accurate

refs #3598

twisted-trac commented 15 years ago
exarkun's avatar @exarkun commented

(In [25849]) document copyright holder and license of _findlinestarts

refs #3598

twisted-trac commented 15 years ago
exarkun's avatar @exarkun commented

(In [25781]) Branching to 'findlinestarts-3598'

twisted-trac commented 15 years ago
exarkun's avatar @exarkun commented

(In [25782]) use dis.findlinestarts instead of inspect.getsourcelines

refs #3598

twisted-trac commented 15 years ago
exarkun's avatar @exarkun commented

(In [25846]) test edge cases for line number stuff

refs #3598

twisted-trac commented 15 years ago
exarkun's avatar @exarkun commented

(In [25847]) rename single letter variable "w" to something more descriptive

refs #3598

twisted-trac commented 15 years ago
exarkun's avatar @exarkun commented

This is a small part of #3554.

twisted-trac commented 15 years ago
exarkun's avatar @exarkun commented

(In [26041]) explain the getabsfile problem in more detail; change the name of the module used in the renamed-source-file-test to be more clear

refs #3598