third_party/WebKit/LayoutTests/transitions/unprefixed-transform.html - Issue 2578143003: CSS Transitions: Fix flaky test unprefixed-transform.html

Side by Side Diff: third_party/WebKit/LayoutTests/transitions/unprefixed-transform.html

Issue 2578143003: CSS Transitions: Fix flaky test unprefixed-transform.html (Closed)

Patch Set: Created 4 years ago

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

Jump to:

« no previous file with comments | « third_party/WebKit/LayoutTests/TestExpectations ('k') | third_party/WebKit/LayoutTests/transitions/unprefixed-transform-expected.txt » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

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

2 <div class="target">-webkit-transform 10ms</div><br>	2 <meta charset="utf-8">

3 <div class="target">transform 10ms</div><br>	3 <script src="../resources/testharness.js"></script>

4 <div class="target">transform 10ms, -webkit-transform 10ms</div><br>	4 <script src="../resources/testharnessreport.js"></script>

5 <div class="target">-webkit-transform 10ms, transform 10ms</div><br>	5 <div id="container"></div>

6 <div class="target">all 10ms</div><br>

7 <div class="target">all 10ms, -webkit-transform 10ms</div><br>

8 <div class="target">-webkit-transform 10ms, all 10ms</div>

9 <script>	6 <script>

10 if (window.testRunner) {	7 'use strict';

11 testRunner.dumpAsText();

12 testRunner.waitUntilDone();

13 }

14	8

15 document.addEventListener('transitionend', function(e) {	9 var transitionValues = [

16 var result = document.createElement('div');	10 '-webkit-transform 10ms',

17 result.textContent = 'fired transitionend with propertyName: ' + e.propertyNam e;	11 'transform 10ms',

18 e.target.appendChild(result);	12 'transform 10ms, -webkit-transform 10ms',

19 if (window.testRunner) {	13 '-webkit-transform 10ms, transform 10ms',

20 // All events should be fired by next frame.	14 'all 10ms',

21 requestAnimationFrame(function() {	15 'all 10ms, -webkit-transform 10ms',

22 testRunner.notifyDone();	16 '-webkit-transform 10ms, all 10ms'

	17 ];

	18

	19 async_test(t => {

	20 var remaining = transitionValues.length;

	21 document.addEventListener('transitionend', e => {

	22 t.step_func(() => {

	23 assert_equals(e.propertyName, 'transform');
	suzyh_UTC10 (ex-contributor) 2016/12/18 23:47:27 Any other checks it would make sense to do on the Any other checks it would make sense to do on the event, or do we simply want to test that the event fired? Eric Willigers 2016/12/19 06:04:37 Added a few more asserts on event properties. Show quoted text On 2016/12/18 23:47:27, suzyh wrote: > Any other checks it would make sense to do on the event, or do we simply want to > test that the event fired? Added a few more asserts on event properties.
	24 });

	25 remaining -= 1;

	26 if (!remaining)
	suzyh_UTC10 (ex-contributor) 2016/12/18 23:47:27 What happens if we don't get the wrong number of e What happens if we don't get the wrong number of events (too many or too few)? Too few, I guess the test just times out. Too many might be more problematic. Is there an extra assertion we can make to verify we don't hit either of these cases? Eric Willigers 2016/12/19 06:04:37 Done, by waiting a short time so we can detect exc Show quoted text On 2016/12/18 23:47:27, suzyh wrote: > What happens if we don't get the wrong number of events (too many or too few)? > Too few, I guess the test just times out. Too many might be more problematic. Is > there an extra assertion we can make to verify we don't hit either of these > cases? Done, by waiting a short time so we can detect excess events.
	27 t.done();

	28 });

	29

	30 for (let i = 0; i < transitionValues.length; i++) {

	31 const element = document.createElement('div');

	32 container.appendChild(element);

	33 requestAnimationFrame(() => {

	34 element.offsetTop;

	35 element.style.transition = transitionValues[i];

	36 element.style.webkitTransform = 'rotate(360deg)';

	37 element.style.transform = 'rotate(360deg)';

23 });	38 });

24 }	39 }

25 });	40 }, 'transitionend event for unprefixed transform');
	suzyh_UTC10 (ex-contributor) 2016/12/18 23:47:27 If this test is targeted at the unprefixed transfo If this test is targeted at the unprefixed transform, perhaps it would make sense to separate out the parts that are to do with the prefixed property. I can imagine we want to test the various combinations, but I wonder if it would be best to have one test set only element.style.webkitTransform or element.style.transform, not both. What do you think? Optional: If this file continues to have both prefixed and unprefixed transforms, and is mostly testing the transitionend event, consider renaming the file. Eric Willigers 2016/12/19 06:04:37 No, the point of this test is that the page might Show quoted text On 2016/12/18 23:47:27, suzyh wrote: > If this test is targeted at the unprefixed transform, perhaps it would make > sense to separate out the parts that are to do with the prefixed property. I can > imagine we want to test the various combinations, but I wonder if it would be > best to have one test set only element.style.webkitTransform or > element.style.transform, not both. What do you think? > > Optional: If this file continues to have both prefixed and unprefixed > transforms, and is mostly testing the transitionend event, consider renaming the > file. No, the point of this test is that the page might have transitions on prefixed and/or unprefixed transforms, and there will always be one event for the transition, on unprefixed transform. Many public web pages intentionally have 'transform ..., -webkit-transform ...' to support multiple browser versions. suzyh_UTC10 (ex-contributor) 2016/12/19 23:10:52 Ah, making sure that there's only one event makes Show quoted text On 2016/12/19 at 06:04:37, Eric Willigers wrote: > On 2016/12/18 23:47:27, suzyh wrote: > > If this test is targeted at the unprefixed transform, perhaps it would make > > sense to separate out the parts that are to do with the prefixed property. I can > > imagine we want to test the various combinations, but I wonder if it would be > > best to have one test set only element.style.webkitTransform or > > element.style.transform, not both. What do you think? > > > > Optional: If this file continues to have both prefixed and unprefixed > > transforms, and is mostly testing the transitionend event, consider renaming the > > file. > > No, the point of this test is that the page might have transitions on prefixed and/or unprefixed transforms, and there will always be one event for the transition, on unprefixed transform. > > Many public web pages intentionally have 'transform ..., -webkit-transform ...' to support multiple browser versions. Ah, making sure that there's only one event makes sense. I'm still curious whether this test helps us be sure that the events are triggered both by setting "element.style.webkitTransform = 'rotate(360deg)'" and by setting "element.style.transform = 'rotate(360deg)'", or whether it's only one of these statements that is responsible. But that point may not be important; I'll leave that to your discretion. To more clearly document this explanation, please change the test description from "transitionend event for unprefixed transform" to something like "A transition on the prefixed and/or unprefixed transform property causes a single transitionend event for the unprefixed transform property." Eric Willigers 2016/12/20 01:06:15 Test description updated. Show quoted text On 2016/12/19 23:10:52, suzyh wrote: > On 2016/12/19 at 06:04:37, Eric Willigers wrote: > > On 2016/12/18 23:47:27, suzyh wrote: > > > If this test is targeted at the unprefixed transform, perhaps it would make > > > sense to separate out the parts that are to do with the prefixed property. I > can > > > imagine we want to test the various combinations, but I wonder if it would > be > > > best to have one test set only element.style.webkitTransform or > > > element.style.transform, not both. What do you think? > > > > > > Optional: If this file continues to have both prefixed and unprefixed > > > transforms, and is mostly testing the transitionend event, consider renaming > the > > > file. > > > > No, the point of this test is that the page might have transitions on prefixed > and/or unprefixed transforms, and there will always be one event for the > transition, on unprefixed transform. > > > > Many public web pages intentionally have 'transform ..., -webkit-transform > ...' to support multiple browser versions. > > Ah, making sure that there's only one event makes sense. I'm still curious > whether this test helps us be sure that the events are triggered both by setting > "element.style.webkitTransform = 'rotate(360deg)'" and by setting > "element.style.transform = 'rotate(360deg)'", or whether it's only one of these > statements that is responsible. But that point may not be important; I'll leave > that to your discretion. > > To more clearly document this explanation, please change the test description > from "transitionend event for unprefixed transform" to something like "A > transition on the prefixed and/or unprefixed transform property causes a single > transitionend event for the unprefixed transform property." Test description updated.
26

27 onload = function() {

28 var targets = document.querySelectorAll('.target');

29 for (var i = 0; i < targets.length; i++) {

30 targets[i].offsetTop;

31 targets[i].style.transition = targets[i].textContent;

32 targets[i].style.webkitTransform = 'rotate(360deg)';

33 targets[i].style.transform = 'rotate(360deg)';

34 }

35 };

36 </script>	41 </script>

OLD	NEW