third_party/WebKit/LayoutTests/fast/events/mouse-event-buttons-attribute.html - Issue 1775613004: Fix mouse-event-buttons-attribute flakiness

Side by Side Diff: third_party/WebKit/LayoutTests/fast/events/mouse-event-buttons-attribute.html

Issue 1775613004: Fix mouse-event-buttons-attribute flakiness (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 4 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <style>	2 <style>

3 #target {	3 #target {

4 width: 100px;	4 width: 100px;

5 height: 100px;	5 height: 100px;

6 background: blue;	6 background: blue;

7 position: absolute;	7 position: absolute;

8 left: 0px;	8 left: 0px;

9 top: 0px;	9 top: 0px;

10 }	10 }

11 #drag{	11 #drag{

12 position: absolute;	12 position: absolute;

13 left: 0px;	13 left: 0px;

14 top: 100px;	14 top: 100px;

15 }	15 }

16 </style>	16 </style>

17	17 <body>
	Rick Byers 2016/03/09 20:16:16 nit: style says to omit body when it's not necessa nit: style says to omit body when it's not necessary: https://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-... Navid Zolghadr 2016/03/09 20:21:07 Sure. Show quoted text On 2016/03/09 20:16:16, Rick Byers wrote: > nit: style says to omit body when it's not necessary: > https://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-... Sure.
18 <div id="target"></div>

19 <img id="drag" src="resources/greenbox30.png" draggable>

20 <script src="../../resources/js-test.js"></script>	18 <script src="../../resources/js-test.js"></script>

21 <script>	19 <script>

22	20

23 const L = 'leftButton';	21 const L = 'leftButton';

24 const R = 'rightButton';	22 const R = 'rightButton';

25 const M = 'middleButton';	23 const M = 'middleButton';

26	24

27 var targetDiv = document.getElementById('target');

28 var dragImg = document.getElementById('drag');

29 var seqNo = 0;	25 var seqNo = 0;

30 var testSet = [	26 var testSet = [

31 { initialButtons: [], action: doubleClickAction },	27 { initialButtons: [], action: doubleClickAction },

32 { initialButtons: [R], action: clickAction },	28 { initialButtons: [R], action: clickAction },

33 { initialButtons: [M, R], action: clickAction },	29 { initialButtons: [M, R], action: clickAction },

34 { initialButtons: [L, M, R], action: moveAction },	30 { initialButtons: [L, M, R], action: moveAction },

35 { initialButtons: [], action: rightClickAction, showContextMenuOnMouseU p: false },	31 { initialButtons: [], action: rightClickAction, showContextMenuOnMouseU p: false },

36 { initialButtons: [], action: rightClickAction, showContextMenuOnMouseU p: true },	32 { initialButtons: [], action: rightClickAction, showContextMenuOnMouseU p: true },

37 { initialButtons: [L, R], action: wheelAction },	33 { initialButtons: [L, R], action: wheelAction },

38 { initialButtons: [], action: doubleTapAction },	34 { initialButtons: [], action: doubleTapAction },

(...skipping 79 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
118 var showContextMenuOnMouseUp = "";	114 var showContextMenuOnMouseUp = "";

119 if (testItem.showContextMenuOnMouseUp != undefined)	115 if (testItem.showContextMenuOnMouseUp != undefined)

120 showContextMenuOnMouseUp = "and showContextMenuOnMouseUp=" +	116 showContextMenuOnMouseUp = "and showContextMenuOnMouseUp=" +

121 testItem.showContextMenuOnMouseUp + " ";	117 testItem.showContextMenuOnMouseUp + " ";

122	118

123 debug('===== ' + testItem.action.name + ' with initial state=[' + testItem.ini tialButtons + '] '	119 debug('===== ' + testItem.action.name + ' with initial state=[' + testItem.ini tialButtons + '] '

124 + showContextMenuOnMouseUp + '=====');	120 + showContextMenuOnMouseUp + '=====');

125 }	121 }

126	122

127 function init() {	123 function init() {

	124 var targetDiv = document.getElementById('target');

	125 var dragImg = document.getElementById('drag');

128 var eventList = ['dblclick', 'click', 'mousedown', 'mouseup', 'mousemove', 'mo useenter', 'mouseleave', 'mouseover', 'mouseout', 'mousewheel', 'dragstart', 'dr agend', 'dragenter', 'dragleave', 'dragover', 'drag', 'contextmenu'];	126 var eventList = ['dblclick', 'click', 'mousedown', 'mouseup', 'mousemove', 'mo useenter', 'mouseleave', 'mouseover', 'mouseout', 'mousewheel', 'dragstart', 'dr agend', 'dragenter', 'dragleave', 'dragover', 'drag', 'contextmenu'];

129 eventList.forEach(function(eventName) {	127 eventList.forEach(function(eventName) {

130 targetDiv.addEventListener(eventName, function(event) {	128 targetDiv.addEventListener(eventName, function(event) {

131 debug(++seqNo + ". targetDiv received " + event.type + " buttons: " + even t.buttons);	129 debug(++seqNo + ". targetDiv received " + event.type + " buttons: " + even t.buttons);

132 });	130 });

133 dragImg.addEventListener(eventName, function(event) {	131 dragImg.addEventListener(eventName, function(event) {

134 debug(++seqNo + ". dragImg received " + event.type + " buttons: " + event. buttons);	132 debug(++seqNo + ". dragImg received " + event.type + " buttons: " + event. buttons);

135 });	133 });

136 });	134 });

137 }	135 }

138	136

139 function runTests() {	137 function runTests() {

140 for (var i = 0; i < testSet.length; i++) {	138 init();

141 printTestInfo(testSet[i]);	139 if(window.eventSender) {

	140 for (var i = 0; i < testSet.length; i++) {

	141 printTestInfo(testSet[i]);

142	142

143 // Reset the state for the new test	143 // Reset the state for the new test

144 seqNo = 0;	144 seqNo = 0;

145 eventSender.setMouseButtonState(-1, testSet[i].initialButtons);	145 eventSender.setMouseButtonState(-1, testSet[i].initialButtons);

146 internals.settings.setShowContextMenuOnMouseUp(testSet[i].showContextMenuOnM ouseUp);	146 internals.settings.setShowContextMenuOnMouseUp(testSet[i].showContextMenuO nMouseUp);

147	147

148 testSet[i].action();	148 testSet[i].action();

149	149

150 // Reset the mouse position	150 // Reset the mouse position

151 eventSender.mouseMoveTo(-1, -1);	151 eventSender.mouseMoveTo(-1, -1);

152	152

153 debug('');	153 debug('');

	154 }

	155 finishJSTest();

154 }	156 }

155 }	157 }

156	158

157 init();	159 if (window.eventSender) {

158 if (window.eventSender)	160 testRunner.waitUntilDone();

159 runTests();	161 window.jsTestIsAsync = true;
	Rick Byers 2016/03/09 20:16:16 does this actually need to be an async test? IIRC does this actually need to be an async test? IIRC non-async tests are considered complete after all onload handlers have finished running. So moving the runtests call to the img onload is perhaps all you need here. Navid Zolghadr 2016/03/09 20:21:07 The problem was that I would get one "PASS success Show quoted text On 2016/03/09 20:16:16, Rick Byers wrote: > does this actually need to be an async test? IIRC non-async tests are > considered complete after all onload handlers have finished running. So moving > the runtests call to the img onload is perhaps all you need here. The problem was that I would get one "PASS successfullyParsed is true TEST COMPLETE" at the beginning of output file. Is that okay to add that to the -expected file or is there another way of getting rid of that beside making the test async? mustaq 2016/03/09 21:38:01 I thought too-early TEST COMPLETE message is a sig Show quoted text On 2016/03/09 20:21:07, Navid Zolghadr wrote: > On 2016/03/09 20:16:16, Rick Byers wrote: > > does this actually need to be an async test? IIRC non-async tests are > > considered complete after all onload handlers have finished running. So > moving > > the runtests call to the img onload is perhaps all you need here. > > The problem was that I would get one > "PASS successfullyParsed is true > > TEST COMPLETE" > > at the beginning of output file. Is that okay to add that to the -expected file > or is there another way of getting rid of that beside making the test async? I thought too-early TEST COMPLETE message is a sign of "asyncness". I remember encountering it before, and marked the test as async (w/o waitUntilDone which seems redundant here: js-test.js:102). I didn't try not-marking-async, worth a try though. Navid Zolghadr 2016/03/10 13:57:50 I'm a bit confused of what you asked me to do. I t Show quoted text On 2016/03/09 21:38:01, mustaq wrote: > On 2016/03/09 20:21:07, Navid Zolghadr wrote: > > On 2016/03/09 20:16:16, Rick Byers wrote: > > > does this actually need to be an async test? IIRC non-async tests are > > > considered complete after all onload handlers have finished running. So > > moving > > > the runtests call to the img onload is perhaps all you need here. > > > > The problem was that I would get one > > "PASS successfullyParsed is true > > > > TEST COMPLETE" > > > > at the beginning of output file. Is that okay to add that to the -expected > file > > or is there another way of getting rid of that beside making the test async? > > I thought too-early TEST COMPLETE message is a sign of "asyncness". I remember > encountering it before, and marked the test as async (w/o waitUntilDone which > seems redundant here: js-test.js:102). > > I didn't try not-marking-async, worth a try though. I'm a bit confused of what you asked me to do. I think this comment is on patchset 1. Can you look at latest patchset and see whether that is what you meant? mustaq 2016/03/10 14:51:12 Yes, I meant patch#2 or even #3 worth a try, to co Show quoted text On 2016/03/10 13:57:50, Navid Zolghadr wrote: > On 2016/03/09 21:38:01, mustaq wrote: > > On 2016/03/09 20:21:07, Navid Zolghadr wrote: > > > On 2016/03/09 20:16:16, Rick Byers wrote: > > > > does this actually need to be an async test? IIRC non-async tests are > > > > considered complete after all onload handlers have finished running. So > > > moving > > > > the runtests call to the img onload is perhaps all you need here. > > > > > > The problem was that I would get one > > > "PASS successfullyParsed is true > > > > > > TEST COMPLETE" > > > > > > at the beginning of output file. Is that okay to add that to the -expected > > file > > > or is there another way of getting rid of that beside making the test async? > > > > I thought too-early TEST COMPLETE message is a sign of "asyncness". I remember > > encountering it before, and marked the test as async (w/o waitUntilDone which > > seems redundant here: js-test.js:102). > > > > I didn't try not-marking-async, worth a try though. > > I'm a bit confused of what you asked me to do. I think this comment is on > patchset 1. Can you look at latest patchset and see whether that is what you > meant? Yes, I meant patch#2 or even #3 worth a try, to confirm if early TEST COMPLETE message is really not an issue. If the test turns out to be flaky again, please mark it as async before anything else.
160 else	162 } else {

161 debug("This test requires eventSender");	163 debug("This test requires eventSender");

	164 }

162	165

163 </script>	166 </script>

	167

	168 <div id="target"></div>

	169 <img id="drag" src="resources/greenbox30.png" onload="runTests()" draggable>

	170 </body>

OLD	NEW

« no previous file with comments | « third_party/WebKit/LayoutTests/TestExpectations ('k') | no next file » | no next file with comments »