Bug 86800

Summary: [NRWT] REGRESSION(r116021): Assertion when running with --no-http
Product: WebKit Reporter: Raphael Kubo da Costa (:rakuco) <rakuco>
Component: Tools / TestsAssignee: Thiago Marcos P. Santos <tmpsantos>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, ojan, ossy, tmpsantos, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 64491, 78628    
Attachments:
Description Flags
Patch none

Raphael Kubo da Costa (:rakuco)
Reported 2012-05-17 19:52:37 PDT
When running `./Tools/Scripts/run-webkit-tests --efl --debug --no-http' I hit the following assertion: Traceback (most recent call last): File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 457, in <module> sys.exit(main()) File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 452, in main return run(port, options, args) File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 125, in run unexpected_result_count = manager.run() File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 906, in run interrupted, keyboard_interrupted, thread_timings, test_timings, individual_test_timings = self._run_tests(self._test_files_list, result_summary, int(self._options.child_processes)) File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 756, in _run_tests self.start_servers_with_lock() File "/home/rakuco/dev/webkit/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 955, in start_servers_with_lock assert(self._options.http)
Attachments
Patch (1.52 KB, patch)
2012-05-24 03:29 PDT, Thiago Marcos P. Santos 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dirk Pranke
Comment 1 2012-05-17 19:55:13 PDT
Looks like this is caused by including perf tests in the "locked" shard.
Raphael Kubo da Costa (:rakuco)
Comment 2 2012-05-17 19:55:58 PDT
Added in <http://trac.webkit.org/changeset/116021>.
Raphael Kubo da Costa (:rakuco)
Comment 3 2012-05-23 08:22:55 PDT
*** Bug 87267 has been marked as a duplicate of this bug. ***
Tony Chang
Comment 4 2012-05-23 10:15:32 PDT
We should roll out if no one is going to get a chance to investigate this soon.
Dirk Pranke
Comment 5 2012-05-23 13:18:03 PDT
(In reply to comment #4) > We should roll out if no one is going to get a chance to investigate this soon. I'll fix this later today.
Thiago Marcos P. Santos
Comment 6 2012-05-24 03:29:04 PDT
Created attachment 143777 [details] Patch
Adam Barth
Comment 7 2012-05-24 08:39:08 PDT
Comment on attachment 143777 [details] Patch Can we write a test that will catch this problem in the future?
Dirk Pranke
Comment 8 2012-05-24 12:04:38 PDT
This patch looks fine, but I think it might actually be better to revert the change that added the perf tests to the locked shards, since that didn't actually do what Tony thought it did, and I'm not sure if it had the intended effect. Tony, WDYT?
Thiago Marcos P. Santos
Comment 9 2012-05-24 14:22:11 PDT
(In reply to comment #8) > This patch looks fine, but I think it might actually be better to revert the change that added the perf tests to the locked shards, since that didn't actually do what Tony thought it did, and I'm not sure if it had the intended effect. Tony, WDYT? I was the one who wrote the original patch moving perf to locked shards. For the EFL Debug bot at least, it solved the flakiness issue. And yes, I can write utests for it.
Tony Chang
Comment 10 2012-05-24 14:28:27 PDT
(In reply to comment #8) > This patch looks fine, but I think it might actually be better to revert the change that added the perf tests to the locked shards, since that didn't actually do what Tony thought it did, and I'm not sure if it had the intended effect. Tony, WDYT? Either is fine with me. It sounds like there were benefits of this patch for tmpsantos, so I don't mind keeping it. It still ensures that we don't mix http tests with perf tests, right?
Dirk Pranke
Comment 11 2012-05-24 14:33:11 PDT
Oh, sorry, I thought Tony did the original patch. At any rate, the change looks fine so I'll r+ it and if it's valuable to you feel free to land it.
Thiago Marcos P. Santos
Comment 12 2012-05-24 14:38:18 PDT
(In reply to comment #11) > Oh, sorry, I thought Tony did the original patch. > > At any rate, the change looks fine so I'll r+ it and if it's valuable to you feel free to land it. I might need someone with cq+ superpowers then. :)
WebKit Review Bot
Comment 13 2012-05-24 15:03:48 PDT
Comment on attachment 143777 [details] Patch Clearing flags on attachment: 143777 Committed r118421: <http://trac.webkit.org/changeset/118421>
WebKit Review Bot
Comment 14 2012-05-24 15:03:55 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.