Summary: | run-webkit-tests error when running certain subdirectories of layout tests | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adrienne Walker <enne> | ||||||||
Component: | Tools / Tests | Assignee: | Dirk Pranke <dpranke> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | dpranke, enne, eric, mihaip, ojan, tony | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Attachments: |
|
Description
Adrienne Walker
2011-01-14 13:58:12 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. Ah, you're quite right. If I add --force, then tests exist and this doesn't occur. Created attachment 79019 [details]
Patch
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. (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? 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. (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 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? (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. Created attachment 79030 [details]
Patch
(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 on attachment 79030 [details]
Patch
r- to make it clearer that tests for the bug being fixed would be good to have.
Created attachment 79224 [details]
update w/ mihaip's feedback, rebase to tip of tree
Committed r76045: <http://trac.webkit.org/changeset/76045> |