Repository navigation
use unittest for test_logging #46081
Description
| BPO | 1740 |
|---|---|
| Nosy | @brettcannon, @pitrou, @tiran |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/brettcannon'
closed_at = <Date 2008-03-03.00:40:02.027>
created_at = <Date 2008-01-06.01:40:05.490>
labels = ['tests', 'performance']
title = 'use unittest for test_logging'
updated_at = <Date 2008-03-03.00:40:30.821>
user = 'https://github.com/pitrou'bugs.python.org fields:
activity = <Date 2008-03-03.00:40:30.821>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2008-03-03.00:40:02.027>
closer = 'brett.cannon'
components = ['Tests']
creation = <Date 2008-01-06.01:40:05.490>
creator = 'pitrou'
dependencies = []
files = ['9073', '9074', '9522', '9548']
hgrepos = []
issue_num = 1740
keywords = ['patch']
message_count = 13.0
messages = ['59348', '59349', '59350', '62502', '62507', '62822', '62971', '62995', '63025', '63066', '63069', '63198', '63199']
nosy_count = 4.0
nosy_names = ['brett.cannon', 'pitrou', 'therve', 'christian.heimes']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue1740'
versions = ['Python 2.6']
Activity
pitrou commentedon Jan 6, 2008
Lib/test.test_logging.py doesn't use unittest. Here is a patch (against
SVN trunk) to fix the problem.
tiran commentedon Jan 6, 2008
Thanks! :)
I quick review hasn't shown any larger issues but it need a proper
review before it can be submitted.
You probably should save _levelNames, _loggerClass, root, Logger.root
and Logger.manager in setUp and reset the objects in tearDown.
pitrou commentedon Jan 6, 2008
Ok here is an updated patch with slightly more complete setUp/tearDown
semantics. I don't backup all the objects though since some of them
(e.g. _loggerClass) aren't touched by the tests.
pitrou commentedon Feb 17, 2008
Hmm, apparently another patch was committed in rev 60872.
brettcannon commentedon Feb 17, 2008
Yeah, that's my fault. I forgot to search the issue tracker first before
committing the GHOP rewrite.
I will do a review of this patch and see which version is better.
pitrou commentedon Feb 23, 2008
Here is an updated patch, it should apply fine against trunk.
brettcannon commentedon Feb 25, 2008
I am attaching a reviewed version of the patch. It had some major PEP-8
violations that I had to clean up. I also moved over to the usage of
test.test_support.captured_stdout(). Otherwise it looks good.
I am going to wait a little while in hopes someone else can at least
take a look to make sure that the tests are proper and thorough since it
is a complete rewrite.
pitrou commentedon Feb 25, 2008
Out of curiosity, what were the PEP-8 violations? Usually I try to
respect the coding guidelines.
brettcannon commentedon Feb 26, 2008
On Mon, Feb 25, 2008 at 11:39 AM, Antoine Pitrou <[email protected]> wrote:
There were three that pervaded the code.
A newline after the opening docstring quotes::
"""
docstring
"""
is bad.
Use of camelCase for methods and variables is bad.
The doc string for classes is supposed to have a blank line between
the class definition line and the docstring itself.
therve commentedon Feb 27, 2008
I have just one comment: I have recently modified the initialization of
LogRecordSocketReceiver to not use a stupid loop to get a free port, but
instead use port 0 and get the port number once bound. I think this
modification should stay.
tiran commentedon Feb 27, 2008
Thomas Herve wrote:
Most likely the student, who submitted the patch during the GHOP
contest, didn't know about the feature. ;) But yeah, the mod should stay
indeed.
Christian
brettcannon commentedon Mar 3, 2008
Committed in r61189 on the trunk. Thanks, Antoine!
brettcannon commentedon Mar 3, 2008
Oh, and thanks Thomas for the port change. I made sure to keep it.