Bug 52478 - run-webkit-tests error when running certain subdirectories of layout tests
Summary: run-webkit-tests error when running certain subdirectories of layout tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-14 13:58 PST by Adrienne Walker
Modified: 2011-01-18 12:32 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.46 KB, patch)
2011-01-14 15:55 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff
Patch (4.64 KB, patch)
2011-01-14 16:58 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff
update w/ mihaip's feedback, rebase to tip of tree (4.79 KB, patch)
2011-01-17 15:22 PST, Dirk Pranke
mihaip: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrienne Walker 2011-01-14 13:58:12 PST
On Linux, if I run "new-run-webkit-tests --debug --chromium LayoutTests/compositing" then I get the following error:

Preparing tests ...Traceback (most recent call last):
  File "/usr/local/google/home/enne/chrome/src/third_party/WebKit/Tools/Scripts/new-run-webkit-tests", line 38, in <module>
    sys.exit(run_webkit_tests.main())
  File "/usr/local/google/home/enne/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 430, in main
    return run(port_obj, options, args)
  File "/usr/local/google/home/enne/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 117, in run
    num_unexpected_results = runner.run(result_summary)
  File "/usr/local/google/home/enne/chrome/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py", line 645, in run
    assert(len(self._test_files))
AssertionError

This does not occur if I run against LayoutTests/fast/canvas/webgl (and any number of other subdirectories).  Oddly, if I generate a text list containing all of the html files in the compositing directory (recursively) and then use --test-file to specify it, then I get the same error as above.
Comment 1 Dirk Pranke 2011-01-14 15:49:06 PST
This is happening "by design", sort of. You must have lines in your test expectations file that skips the compositing directory. When you list files individually, those skips get overridden.

The tool shouldn't crash, though. I'll add a patch to fix that.
Comment 2 Adrienne Walker 2011-01-14 15:54:09 PST
Ah, you're quite right.  If I add --force, then tests exist and this doesn't occur.
Comment 3 Dirk Pranke 2011-01-14 15:55:24 PST
Created attachment 79019 [details]
Patch
Comment 4 Dirk Pranke 2011-01-14 15:56:53 PST
Note that passing a  directory is different than an explicit list of test files because we assume you still want to skip the files that should be skipped in the directory unless told not to.

You can also override the skipping by using --force.
Comment 5 Adrienne Walker 2011-01-14 16:01:20 PST
(In reply to comment #4)
> Note that passing a  directory is different than an explicit list of test files because we assume you still want to skip the files that should be skipped in the directory unless told not to.

Using --test-list had the same behavior as passing a directory for me.  Are you saying that's not intended?
Comment 6 Dirk Pranke 2011-01-14 16:22:24 PST
Looks like there might be a separate bug as well ... when you specify arguments on the command line we do you the courtesy of stripping off the "LayoutTests/" prefix. We don't for lines listed in --test-file . So you were getting the same result through a slightly different code path, I think.
Comment 7 Mihai Parparita 2011-01-14 16:26:01 PST
(In reply to comment #6)
> Looks like there might be a separate bug as well ... when you specify arguments on the command line we do you the courtesy of stripping off the "LayoutTests/" prefix. We don't for lines listed in --test-file . So you were getting the same result through a slightly different code path, I think.

I thought we had stopped stripping the LayoutTests/ prefix (http://trac.webkit.org/changeset/71160), for consistency with ORWT.
Comment 8 Mihai Parparita 2011-01-14 16:27:02 PST
Comment on attachment 79019 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:394
> +        if not len(self._test_files):

Unit test?
Comment 9 Dirk Pranke 2011-01-14 16:30:12 PST
(In reply to comment #7)
> (In reply to comment #6)
> > Looks like there might be a separate bug as well ... when you specify arguments on the command line we do you the courtesy of stripping off the "LayoutTests/" prefix. We don't for lines listed in --test-file . So you were getting the same result through a slightly different code path, I think.
> 
> I thought we had stopped stripping the LayoutTests/ prefix (http://trac.webkit.org/changeset/71160), for consistency with ORWT.

That is the aforementioned bug. Ojan fixed it in one place and missed it in the other. I will fix that and add a couple of unit tests.
Comment 10 Dirk Pranke 2011-01-14 16:58:58 PST
Created attachment 79030 [details]
Patch
Comment 11 Mihai Parparita 2011-01-14 17:03:35 PST
(In reply to comment #9)
> That is the aforementioned bug. Ojan fixed it in one place and missed it in the other. I will fix that and add a couple of unit tests.

Ah, makes sense now.

What about a unit test for the original report (that you get a crash if trying to run only skipped tests)?
Comment 12 Mihai Parparita 2011-01-17 12:58:38 PST
Comment on attachment 79030 [details]
Patch

r- to make it clearer that tests for the bug being fixed would be good to have.
Comment 13 Dirk Pranke 2011-01-17 15:22:26 PST
Created attachment 79224 [details]
update w/ mihaip's feedback, rebase to tip of tree
Comment 14 Dirk Pranke 2011-01-18 12:32:34 PST
Committed r76045: <http://trac.webkit.org/changeset/76045>