Bug 99632

Summary: [chromium] Re-enable gesture highlight layout tests, fix GestureLongPress regression.
Product: WebKit Reporter: W. James MacLean <wjmaclean>
Component: New BugsAssignee: W. James MacLean <wjmaclean>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aelias, enne, jamesr, rjkroege, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

W. James MacLean
Reported 2012-10-17 13:36:45 PDT
[chromium] Re-enable gesture highlight layout tests, fix GestureLongPress regression.
Attachments
Patch (966.74 KB, patch)
2012-10-17 13:41 PDT, W. James MacLean
no flags
Patch (3.85 KB, patch)
2012-10-18 14:12 PDT, W. James MacLean
no flags
W. James MacLean
Comment 1 2012-10-17 13:41:36 PDT
W. James MacLean
Comment 2 2012-10-18 13:57:37 PDT
Comment on attachment 169248 [details] Patch Just realized the new baselines are wrong due to the Precise fonts being different ... will revise patch and re-upload.
W. James MacLean
Comment 3 2012-10-18 14:12:21 PDT
WebKit Review Bot
Comment 4 2012-10-18 16:39:25 PDT
Comment on attachment 169472 [details] Patch Clearing flags on attachment: 169472 Committed r131823: <http://trac.webkit.org/changeset/131823>
WebKit Review Bot
Comment 5 2012-10-18 16:39:28 PDT
All reviewed patches have been landed. Closing bug.
Adam Barth
Comment 6 2012-10-25 22:21:38 PDT
This patch did not appear to fix https://bugs.webkit.org/show_bug.cgi?id=96951 despite removing the lines in TestExpectations that reference that bug.
W. James MacLean
Comment 7 2012-10-26 10:53:46 PDT
(In reply to comment #6) > This patch did not appear to fix https://bugs.webkit.org/show_bug.cgi?id=96951 despite removing the lines in TestExpectations that reference that bug. I don't quite follow. Are you saying the chromium-linux link highlight tests aren't being run on the bots? They do run locally on my linux machine via new-run-webkit-tests.
Adam Barth
Comment 8 2012-10-26 14:07:29 PDT
> I don't quite follow. Are you saying the chromium-linux link highlight tests aren't being run on the bots? They do run locally on my linux machine via new-run-webkit-tests. You unskipped the tests without fixing Bug 96951. I noticed that Bug 96951 was occurring again, and I skipped the tests again. Please do not re-enable these tests without fixing Bug 96951. It slows down the entire project.
W. James MacLean
Comment 9 2012-10-29 06:01:20 PDT
(In reply to comment #8) > > I don't quite follow. Are you saying the chromium-linux link highlight tests aren't being run on the bots? They do run locally on my linux machine via new-run-webkit-tests. > > You unskipped the tests without fixing Bug 96951. I noticed that Bug 96951 was occurring again, and I skipped the tests again. Please do not re-enable these tests without fixing Bug 96951. It slows down the entire project. Based on our investigation the flakiness was not limited to (nor caused by) the gesture highlight tests. It appears to be some state the compositor gets into, and seemingly on another (possibly passing) test. I'm going to guess there are still other compositor tests failing in the same way, as this was the pattern before. If that's the case, then by your logic above we should be disabling all the compositor tests that exhibit flake. Are other compositor tests failing?
Adam Barth
Comment 10 2012-10-29 12:20:30 PDT
Here's what I know: 1) If we run these tests, we get large queuing delays in the EWS. 2) If we skip these tests, we do not get large queuing delays. 3) Large queuing delays are not acceptable. I'd love to enable these tests, but we cannot enable them if they cause large queuing delays. It might well be that the problem is with the compositor and affects other tests, but that doesn't mean that we can enable the tests.
W. James MacLean
Comment 11 2012-10-29 14:08:50 PDT
(In reply to comment #10) > Here's what I know: > > 1) If we run these tests, we get large queuing delays in the EWS. > 2) If we skip these tests, we do not get large queuing delays. > 3) Large queuing delays are not acceptable. > > I'd love to enable these tests, but we cannot enable them if they cause large queuing delays. It might well be that the problem is with the compositor and affects other tests, but that doesn't mean that we can enable the tests. Fair enough, but now we have *zero* test coverage on this feature. Perhaps it would have been better to see if there was some critical subset of these tests that might stay on, while we disable the rest, so we can have the best of both worlds? Perhaps if we need this many tests turned off, the load can be distributed so that it's not just highlight tests being turned off.
Adam Barth
Comment 12 2012-10-29 14:17:18 PDT
I sympathize, but I'd rather we just fixed the underlying problem. These tests have caused this problem since they were introduced.
W. James MacLean
Comment 13 2012-10-29 15:45:41 PDT
(In reply to comment #12) > I sympathize, but I'd rather we just fixed the underlying problem. These tests have caused this problem since they were introduced. I'd rather we fixed the underlying problem too, and while I had some resources available I was attempting to do so. But I no longer have anyone to work on the problem, myself included.
Note You need to log in before you can comment on or make changes to this bug.