Bug 36591

Summary: test-webkitpy: should automatically detect all unit test files in webkitpy
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
none
Proposed patch 2
none
Proposed patch 3 abarth: review+, cjerdonek: commit-queue-

Description Chris Jerdonek 2010-03-25 04:02:24 PDT
This should make it unnecessary to have and maintain all of the unittests.py files.

test-webkitpy should be able to traverse webkitpy/, look for all files of the form *_unittest.py, and then import * from those files and call unittest.main().
Comment 1 Chris Jerdonek 2010-03-25 08:09:33 PDT
See this page:

http://technogeek.org/python-module.html

Since we need to execute statements of the form "from ..._unittest import *", we may want to use the pattern below as suggested in the link above:

import_string = "from string import ascii_letters"
exec import_string
Comment 2 Eric Seidel (no email) 2010-03-25 13:36:02 PDT
No need for "exec".  We can just import the module and then use defs() to grab all the classes and import those, no?

Or maybe there is some piece of http://docs.python.org/library/imp.html to use?
Comment 3 Chris Jerdonek 2010-03-25 13:44:12 PDT
(In reply to comment #2)
> No need for "exec".  We can just import the module and then use defs() to grab
> all the classes and import those, no?

Yeah, there should also be ways without exec().

> Or maybe there is some piece of http://docs.python.org/library/imp.html to use?

Here is something else to look at:

http://docs.python.org/library/unittest.html#unittest.TestLoader.loadTestsFromModule
Comment 4 Chris Jerdonek 2010-03-27 00:02:28 PDT
Created attachment 51815 [details]
Proposed patch

I can delete all of the *_unittest files in a subsequent patch.
Comment 5 Chris Jerdonek 2010-03-27 00:15:52 PDT
Created attachment 51816 [details]
Proposed patch 2

Documented the call to unittest.main().
Comment 6 Adam Barth 2010-03-27 09:59:18 PDT
abarth: cjerdonek: so this is good
abarth: but i think you used to be able to specify a single test
abarth: on the command line
abarth: and run just that one
abarth: this will always run them all
Comment 7 Chris Jerdonek 2010-03-27 10:03:52 PDT
(In reply to comment #6)
> abarth: cjerdonek: so this is good
> abarth: but i think you used to be able to specify a single test
> abarth: on the command line
> abarth: and run just that one
> abarth: this will always run them all

cjerdonek: i preserved that but then removed it
[09:58am] abarth: oh, if its easy
[09:58am] abarth: we should support that
[09:59am] cjerdonek: abarth: i'll put it back it then, i didn't think anyone was actually using that
Comment 8 Chris Jerdonek 2010-03-27 10:35:52 PDT
Created attachment 51832 [details]
Proposed patch 3

Reinstated the ability to run a subset of the tests using command-line arguments.
Comment 9 Adam Barth 2010-03-27 15:14:29 PDT
Comment on attachment 51832 [details]
Proposed patch 3

Looks good.  I don't quite understand how we still have the ability to filter tests, but I believe you.
Comment 10 Chris Jerdonek 2010-03-27 15:29:07 PDT
(In reply to comment #9)
> (From update of attachment 51832 [details])
> Looks good.  I don't quite understand how we still have the ability to filter
> tests, but I believe you.

Thanks, Adam!

It's pretty simple.  If sys.argv contains a non-flag argument at the end, then it bypasses auto-detection and does what it normally does:

+        if len(sys_argv) > 1 and not sys_argv[-1].startswith("-"):
+            # Then explicit modules or test names were provided, which
+            # the unittest module is equipped to handle.
+            unittest.main(module=None)
+            # No need to return since unitttest.main() exits.
+
+        # Otherwise, auto-detect all unit tests.
...
Comment 11 Chris Jerdonek 2010-03-27 15:38:15 PDT
Comment on attachment 51832 [details]
Proposed patch 3

Going to land this manually.
Comment 12 Chris Jerdonek 2010-03-27 16:02:59 PDT
Committed via webkit-patch (somewhat): http://trac.webkit.org/changeset/56671

(Didn't quite work out.  Key-chain issues again.  Frustrating because I just tried updating my keychain info using the KeyChain Access app...)


Logging in as chris.jerdonek@gmail.com...
Bugzilla login failed: Invalid Username Or Password
Reading Keychain for bugs.webkit.org account and password.  Click "Allow" to continue...
Logging in as chris.jerdonek@gmail.com...
Traceback (most recent call last):
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkit-patch", line 70, in <module>
    main()
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkit-patch", line 65, in main
    WebKitPatch(__file__).main()
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py", line 302, in main
    result = command.check_arguments_and_execute(options, args, self)
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkitpy/tool/multicommandtool.py", line 113, in check_arguments_and_execute
    return self.execute(options, args, tool) or 0
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 43, in execute
    self._sequence.run_and_handle_errors(tool, options, self._prepare_state(options, args, tool))
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py", line 66, in run_and_handle_errors
    self._run(tool, options, state)
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py", line 60, in _run
    step(tool, options).run(state)
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py", line 51, in run
    self._tool.bugs.close_bug_as_fixed(bug_id, comment_text)
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py", line 750, in close_bug_as_fixed
    self.authenticate()
  File "/Users/chris_g4/dev/WebKit/trunk-git/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py", line 516, in authenticate
    raise Exception(errorMessage)
Exception: Bugzilla login failed: Invalid Username Or Password