Bug 71234 - webkitpy tests depend to much on the user's environment
Summary: webkitpy tests depend to much on the user's environment
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-31 13:31 PDT by Eric Seidel (no email)
Modified: 2011-10-31 13:51 PDT (History)
4 users (show)

See Also:


Attachments
Patch (17.77 KB, patch)
2011-10-31 13:33 PDT, Eric Seidel (no email)
dpranke: review+
dpranke: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2011-10-31 13:31:23 PDT
webkitpy tests depend too much on the user's environment
Comment 1 Eric Seidel (no email) 2011-10-31 13:33:13 PDT
Created attachment 113080 [details]
Patch
Comment 2 Dirk Pranke 2011-10-31 13:37:45 PDT
Did you just change the subject to be grammatically incorrect? :)
Comment 3 Dirk Pranke 2011-10-31 13:43:08 PDT
Comment on attachment 113080 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=113080&action=review

change looks basically good, with a few nits.

> Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py:187
> +        # FIXME: Note we do not pass a filesystem as the "test port" magically suplies its own mock filesystem.

Why add the FIXME here?

> Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py:116
> +        port = self.make_port(executive=executive)

The whole point of this test is that we're actually testing the implementation of wdiff, so we have to use a real executive. This should be renamed to integration_test_run_wdiff() instead (I think this test predates the concept of integration tests).
Comment 4 Eric Seidel (no email) 2011-10-31 13:50:07 PDT
Thanks.  Updated per your comments.
Comment 5 Eric Seidel (no email) 2011-10-31 13:51:05 PDT
Committed r98877: <http://trac.webkit.org/changeset/98877>