Bug 36429

Summary: test-webkitpy: Add support for a flag that enables verbose logging
Product: WebKit Reporter: Chris Jerdonek <cjerdonek>
Component: Tools / TestsAssignee: Chris Jerdonek <cjerdonek>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cjerdonek, eric, hamaji, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Proposed patch
cjerdonek: commit-queue-
Proposed patch 2
eric: review-
Proposed patch 3 abarth: review+, cjerdonek: commit-queue-

Description Chris Jerdonek 2010-03-21 14:48:17 PDT
Passing "--debug" should set the logging level to logging.DEBUG instead of logging.INFO.
Comment 1 Chris Jerdonek 2010-03-22 04:57:57 PDT
Created attachment 51277 [details]
Proposed patch

This is preparatory work for revisiting the autoinstall rewrite, now that the versioning stuff is all worked out now:

https://bugs.webkit.org/show_bug.cgi?id=35163
Comment 2 Chris Jerdonek 2010-03-22 15:51:12 PDT
Created attachment 51366 [details]
Proposed patch 2

Changed to using a try-except block instead of using a version check in autoinstalled/__init__.py since it is cleaner.

Also FYI, I found a bug in Python 2.6 while working on this patch (which they have now fixed):

http://bugs.python.org/issue8200
Comment 3 Shinichiro Hamaji 2010-03-25 00:24:57 PDT
I think it's better to use --verbose instead of --debug as --debug usually means "debug build" in WebKit. Same discussion was done in Bug 36100. Bug 36521 is also related.
Comment 4 Eric Seidel (no email) 2010-03-25 00:42:04 PDT
Comment on attachment 51366 [details]
Proposed patch 2

--versbose, or --log-level=debug
Comment 5 Chris Jerdonek 2010-03-25 03:14:56 PDT
FYI, --verbose is already taken by Python's unittest module, so we will need to use a different flag name:

> test-webkitpy --help
[INFO] test-webkitpy: Suppressing most webkitpy logging while running unit tests.
Usage: test-webkitpy [options] [test] [...]

Options:
  -h, --help       Show this message
  -v, --verbose    Verbose output
  -q, --quiet      Minimal output


Also, I do think the functionality in this report should be distinct from what unittest's --verbose flag already does, which is display a line for every unit test.  In other words, I don't think we should use the existing flag to do both.
Comment 6 Chris Jerdonek 2010-03-27 13:22:36 PDT
Created attachment 51842 [details]
Proposed patch 3
Comment 7 Chris Jerdonek 2010-03-27 13:33:11 PDT
(In reply to comment #6)
> Created an attachment (id=51842) [details]
> Proposed patch 3

If you don't like my choice to use --verbose-logging, then how about --verbose-webkit to distinguish from the unittest module's --verbose flag?  I chose --verbose-logging because it relates to Python's logging module.

I'd prefer not to expose a --logging-level flag since that seems like over-kill.
Comment 8 Adam Barth 2010-03-31 20:30:30 PDT
Comment on attachment 51842 [details]
Proposed patch 3

I'm not wild about mucking around with argv so much, but I don't think we have any better solutions at the moment.

I don't really understand Python logging, but if the code does what it says, that's great.
Comment 9 Chris Jerdonek 2010-03-31 20:38:48 PDT
(In reply to comment #8)

Thanks!

> (From update of attachment 51842 [details])
> I'm not wild about mucking around with argv so much, but I don't think we have
> any better solutions at the moment.

Yeah, I'm with you.  This is an odd case since test-webkitpy wraps unittest.main().  It's along the same lines as your --all hack.

There may not be an elegant solution, unfortunately.  Maybe the best we can do is re-implementing unittest.main's argument parser so we can get command-line help working for the command options we added, etc.

> I don't really understand Python logging, but if the code does what it says,
> that's great.

It's really not too hard.  It's just that test-webkitpy is a bit trickier than the others since we want to avoid rendering to the screen the log messages that occur as a side effect of executing code.
Comment 10 Adam Barth 2010-03-31 20:40:54 PDT
I didn't (and don't) like the --all thing either.  I just needed to solve that painpoint somehow.  :(
Comment 11 Chris Jerdonek 2010-03-31 20:47:58 PDT
(In reply to comment #10)
> I didn't (and don't) like the --all thing either.  I just needed to solve that
> painpoint somehow.  :(

Yeah, we'll solve them both together (--all and --verbose-logging) when the time comes...
Comment 12 Chris Jerdonek 2010-03-31 21:23:33 PDT
Committed:

http://trac.webkit.org/changeset/56893