Bug 63597 – nrwt: make the code be consistent about using test names instead of filenames or paths

• Component: New Bugs
• Status: RESOLVED FIXED

[Dirk Pranke]

nrwt: make the code be consistent about using test names instead of filenames or paths

[Dirk Pranke]

Created attachment 99027 [details]
Patch

[Dirk Pranke]

For your consideration ... I think this change makes most of the NRWT code cleaner and clearer, but I'm not wedded to it.

WDYT?

[Dirk Pranke]

Oh, I should not that while this patch will work as written, it probably still needs some integration tests to make sure we're not breaking anything on windows ports.

[WebKit Review Bot]

Comment on attachment 99027 [details]
Patch

Attachment 99027 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8958084

[WebKit Review Bot]

Created attachment 99028 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Dirk Pranke]

Created attachment 99421 [details]
Patch

[Dirk Pranke]

it appears that everything works on all the ports, so either the existing tests were good enough for windows or I got lucky. Still need to verify which :)

[Eric Seidel (no email)]

Comment on attachment 99421 [details]
Patch

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

LGTM.  Please make some effort to standardize on test_name where possible before landing.  Obviously don't go crazy changing all sorts of existing code, but certainly the new stuff should be the same name everywehre.

> Tools/ChangeLog:12
> +        It can be hard to figure out where in the code we're assuming
> +        test names are listed as unix-style relative filenames and where
> +        they are either absolute paths or relative paths following the
> +        host filesystem convention.

I agree!

> Tools/ChangeLog:17
> +        This patch changes things so that everything outside of the
> +        Port object uses (and must assume) unix-style relative
> +        filenames (with one exception, which is specifying host-local
> +        filenames as a list of test arguments on the command line).

Yay!

> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:107
> +        self.test = test

I think using test_name consistently would be more clear than "test".  Reading the code I might think "test" was some sort of test object.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:471
> +    def normalize_test_name(self, test):

You use test_name here, but "test" and testname other places.  I think we should standardize on test_name everywhere for clarity.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:923
> +          test: name of the test

If this was test_name we wouldn't even need this explanation. :)

[Dirk Pranke]

(In reply to comment #8)
> (From update of attachment 99421 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=99421&action=review
> 
> LGTM.  Please make some effort to standardize on test_name where possible before landing.  Obviously don't go crazy changing all sorts of existing code, but certainly the new stuff should be the same name everywehre.

Easily done. Thanks!

[Dirk Pranke]

Created attachment 99900 [details]
rebase to HEAD after driver split-out, rename test to test_name where possible

[Dirk Pranke]

Committed r90520: <http://trac.webkit.org/changeset/90520>

[Eric Seidel (no email)]

Comment on attachment 99900 [details]
rebase to HEAD after driver split-out, rename test to test_name where possible

Cleared review? from attachment 99900 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).