WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
35055
SingleTestThread and TestShellThread should share more code
https://bugs.webkit.org/show_bug.cgi?id=35055
Summary
SingleTestThread and TestShellThread should share more code
Eric Seidel (no email)
Reported
2010-02-17 13:40:36 PST
SingleTestThread and TestShellThread should share more code
Attachments
Patch
(11.51 KB, patch)
2010-02-17 13:41 PST
,
Eric Seidel (no email)
abarth
: review+
commit-queue
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1
2010-02-17 13:41:26 PST
Created
attachment 48933
[details]
Patch
Eric Seidel (no email)
Comment 2
2010-02-17 13:49:49 PST
I did this because I was looking at adding support for dumping out the "list of prior test" for any test which fails. Many of our current failures are caused by test interactions. Knowing which tests were run leading up to a specific failure will help us narrow down those failures.
Ojan Vafai
Comment 3
2010-02-17 13:53:52 PST
Comment on
attachment 48933
[details]
Patch
> +# FIXME: Why do we need a separate class to handle --singly? Can't we just use many > +# TestShellThreads and only feed each one test? > +class SingleTestThread(TestThread):
Actually, now that I look more closely at this, I'm not even sure we need that. What's weird right now is that SingleTestThread is spawned from TestShellThread. It's not clear to me why _run_test_singly doesn't just kill testshell/drt and then start it up again. Anyways, it's a FIXME. Otherwise, LGTM.
Dirk Pranke
Comment 4
2010-02-17 17:10:10 PST
please reformat this to stay within 80 columns as per the PEP 8 style guide :) I think this patch is safe, but is basically rearranging ugly code. You put a FIXME in somewhere to just collapse the two classes, and I agree that that would probably be a better way to fix this. Is there some reason you're not doing that as part of this patch? Also, I believe this --run-singly option is only used by the valgrind bots (presumably to help ensure clean test runs); you might put a comment into the code somewhere to indicate this, so people can understand why we need this option at all.
Adam Barth
Comment 5
2010-02-17 23:58:36 PST
Comment on
attachment 48933
[details]
Patch I'm mostly relying on Ojan and Dirk here, but this looks reasonable to me. Please fix the pep8 nits before landing. (We really need to integrate pep8 into the style elf.)
Eric Seidel (no email)
Comment 6
2010-02-19 16:23:08 PST
Attachment 48933
[details]
was posted by a committer and has review+, assigning to Eric Seidel for commit.
Eric Seidel (no email)
Comment 7
2010-03-24 19:58:54 PDT
Wow. I doubt this still even applies.
Eric Seidel (no email)
Comment 8
2010-03-24 19:59:24 PDT
Comment on
attachment 48933
[details]
Patch We'll see what the cq says.
WebKit Commit Bot
Comment 9
2010-03-25 00:16:53 PDT
Comment on
attachment 48933
[details]
Patch Rejecting patch 48933 from commit-queue. Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Adam Barth', '--force']" exit_code: 1 Last 500 characters of output: itpy/layout_tests/layout_package/test_shell_thread.py Hunk #1 FAILED at 48. Hunk #2 succeeded at 62 (offset 3 lines). Hunk #3 succeeded at 143 (offset 3 lines). Hunk #4 FAILED at 156. Hunk #5 succeeded at 180 (offset 3 lines). Hunk #6 succeeded at 276 with fuzz 1 (offset 3 lines). Hunk #7 succeeded at 287 (offset 3 lines). Hunk #8 succeeded at 412 (offset 3 lines). 2 out of 8 hunks FAILED -- saving rejects to file WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_shell_thread.py.rej Full output:
http://webkit-commit-queue.appspot.com/results/1203028
Eric Seidel (no email)
Comment 10
2010-05-07 20:05:15 PDT
This patch is too out of date to save. I'll redo this at a later date.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug