Bug 37432 – add basic mock unit tests to new-run-webkit-tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Dirk Pranke]

This is a CL to add some very minimal unit tests to new-run-webkit-tests. This should be enough to catch out-and-out syntax errors and so forth.

[Dirk Pranke]

Created attachment 53142 [details]
Patch

[Adam Barth]

Comment on attachment 53142 [details]
Patch

+def real_main(options, args, print_results=True):

This is confusing.  Python tries to trick you into having several levels of main.

+try:
+    d = os.path.dirname(__file__)
+except NameError:
+    d = os.path.dirname(sys.argv[0])
+
+sys.path.append(os.path.abspath(os.path.join(d, '../thirdparty')))

What is this?  We shouldn't be screwing around with sys.path.

+from run_webkit_tests import *

This should be:

from webkitpy.layout_test.run_webkit_tests import real_main, parse_args

Importing * make code very difficult to maintain because it imports all the imports transitively.

+if __name__ == '__main__':
+    unittest.main()

We usually skip this part.  You can run tests individually from test-webkitpy anyway.

[Adam Barth]

Also, wil this test spawn off apache and the websocket server?  It might be a better approach to start testing from the "leaves" up instead of from main down.

[Dirk Pranke]

(In reply to comment #3)
> Also, wil this test spawn off apache and the websocket server?  It might be a
> better approach to start testing from the "leaves" up instead of from main
> down.

No. This test suite (so far) only tests the "test" port, which contains no compiled code and stubs out all of the real work.

[Dirk Pranke]

(In reply to comment #2)
> (From update of attachment 53142 [details])
> +def real_main(options, args, print_results=True):
> 
> This is confusing.  Python tries to trick you into having several levels of
> main.
> 

Agreed. I need to do this temporarily. I've filed bug 37464 to clean this up and am doing that in a separate CL just to make it easy to back out if I've forgotten some downstream reference to this file (I don't think there are any, but just to be sure ...)

> +try:
> +    d = os.path.dirname(__file__)
> +except NameError:
> +    d = os.path.dirname(sys.argv[0])
> +
> +sys.path.append(os.path.abspath(os.path.join(d, '../thirdparty')))
> 
> What is this?  We shouldn't be screwing around with sys.path.

Removed. It looks like y'all have cleaned up the import statements enough so that simply adding WebKitTools/Scripts to PYTHONPATH will allow me to run this file directly.

> 
> +from run_webkit_tests import *
> 
> This should be:
> 
> from webkitpy.layout_test.run_webkit_tests import real_main, parse_args
> 
> Importing * make code very difficult to maintain because it imports all the
> imports transitively.

True. I was writing this in a hurry; it's removed now.

> 
> +if __name__ == '__main__':
> +    unittest.main()
> 
> We usually skip this part.  You can run tests individually from test-webkitpy
> anyway.

You might; I don't. This is a pretty standard convention for Python test files and I'm inclined to keep it.

[Dirk Pranke]

Created attachment 53184 [details]
cleaned-up patch w/ abarth's feedback

[Dirk Pranke]

Created attachment 53193 [details]
revised patch - merged in changes from 37464

[Dirk Pranke]

Created attachment 53195 [details]
update Changelog

[Adam Barth]

Comment on attachment 53195 [details]
update Changelog

+ import run_webkit_tests

Our convention is to use absolute imports from webkitpy.  We still have a bunch of places where we need to clean this up, so it's not a big deal.  Yay tests!

[Dirk Pranke]

(In reply to comment #9)
> (From update of attachment 53195 [details])
> + import run_webkit_tests

Quite correct. Fixed prior to landing the patch.

[Dirk Pranke]

Committed r57622: <http://trac.webkit.org/changeset/57622>