classification
Title: use unittest for test_logging
Type: resource usage Stage:
Components: Tests Versions: Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: brett.cannon, christian.heimes, pitrou, therve
Priority: normal Keywords: patch

Created on 2008-01-06 01:40 by pitrou, last changed 2008-03-03 00:40 by brett.cannon. This issue is now closed.

Files
File name Uploaded Description Edit
logtest.patch pitrou, 2008-01-06 01:40 patch for test_logging.py
logtest.patch pitrou, 2008-01-06 04:48
logtest2.patch pitrou, 2008-02-23 22:21
test_logging.diff brett.cannon, 2008-02-25 05:35 Revised patch against r61058
Messages (13)
msg59348 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-01-06 01:40
Lib/test.test_logging.py doesn't use unittest. Here is a patch (against
SVN trunk) to fix the problem.
msg59349 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-01-06 04:33
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.
msg59350 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-01-06 04:48
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.
msg62502 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-02-17 19:43
Hmm, apparently another patch was committed in rev 60872.
msg62507 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-02-17 22:06
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.
msg62822 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-02-23 22:21
Here is an updated patch, it should apply fine against trunk.
msg62971 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-02-25 05:35
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.
msg62995 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-02-25 19:39
Out of curiosity, what were the PEP 8 violations? Usually I try to
respect the coding guidelines.
msg63025 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-02-26 03:37
On Mon, Feb 25, 2008 at 11:39 AM, Antoine Pitrou <report@bugs.python.org> wrote:
>
>  Antoine Pitrou added the comment:
>
>  Out of curiosity, what were the PEP 8 violations? Usually I try to
>  respect the coding guidelines.

There were three that pervaded the code.

1. A newline after the opening docstring quotes::

    """
    docstring
    """

is bad.

2. Use of camelCase for methods and variables is bad.

3. The doc string for classes is supposed to have a blank line between
the class definition line and the docstring itself.
msg63066 - (view) Author: Thomas Herve (therve) * Date: 2008-02-27 08:35
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.
msg63069 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-02-27 11:09
Thomas Herve wrote:
> 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.

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
msg63198 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-03-03 00:40
Committed in r61189 on the trunk. Thanks, Antoine!
msg63199 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-03-03 00:40
Oh, and thanks Thomas for the port change. I made sure to keep it.
History
Date User Action Args
2008-03-03 00:40:30brett.cannonsetmessages: + msg63199
2008-03-03 00:40:02brett.cannonsetstatus: open -> closed
messages: + msg63198
2008-02-27 11:09:56christian.heimessetmessages: + msg63069
2008-02-27 08:35:37thervesetnosy: + therve
messages: + msg63066
2008-02-26 03:37:54brett.cannonsetmessages: + msg63025
2008-02-25 19:39:18pitrousetmessages: + msg62995
2008-02-25 05:36:05brett.cannonsetfiles: + test_logging.diff
messages: + msg62971
2008-02-23 22:21:25pitrousetfiles: + logtest2.patch
messages: + msg62822
2008-02-17 22:06:56brett.cannonsetassignee: brett.cannon
messages: + msg62507
nosy: + brett.cannon
2008-02-17 19:43:37pitrousetmessages: + msg62502
2008-01-06 04:48:19pitrousetfiles: + logtest.patch
messages: + msg59350
2008-01-06 04:33:38christian.heimessetnosy: + christian.heimes
messages: + msg59349
priority: normal
type: resource usage
keywords: + patch
resolution: accepted
2008-01-06 01:40:05pitroucreate