third_party/WebKit/LayoutTests/media/track/css-cue-for-video-in-shadow.html - Issue 1856383002: Convert css cue track tests from video-test.js to testharness.js based

Side by Side Diff: third_party/WebKit/LayoutTests/media/track/css-cue-for-video-in-shadow.html

Issue 1856383002: Convert css cue track tests from video-test.js to testharness.js based (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 4 years, 8 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

« no previous file with comments | « no previous file | third_party/WebKit/LayoutTests/media/track/css-cue-for-video-in-shadow-2.html » ('j') | third_party/WebKit/LayoutTests/media/track/css-cue-for-video-in-shadow-2.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

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

2 <html>	2 <title>Test that the cue is not styled when video is in a shadow tree and style is in a document.</title>

3 <head>

4 <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
mlamouri (slow - plz ping) 2016/04/06 09:22:58 Why are you removing all that? Having <html>, <hea Why are you removing all that? Having <html>, <head> and <body> wouldn't hurt. Feel free to keep the <script> after body if you only want to have the script run after all the DOM's <body> is constructed. Srirama 2016/04/06 10:21:23 Even this was suggested by philipj and as per codi Show quoted text On 2016/04/06 09:22:58, Mounir Lamouri wrote: > Why are you removing all that? Having <html>, <head> and <body> wouldn't hurt. > Feel free to keep the <script> after body if you only want to have the script > run after all the DOM's <body> is constructed. Even this was suggested by philipj and as per coding guidelines these tags can be omitted (https://www.chromium.org/blink/coding-style/layout-test-style-guidelines), so i am following this style, though keeping them also won't harm as you said.
5 <script src="../media-file.js"></script>	3 <script src="../media-file.js"></script>

6 <!-- TODO(philipj): Convert test to testharness.js. crbug.com/588956

7 (Please avoid writing new tests using video-test.js) -->

8 <script src="../video-test.js"></script>

9 <script src="../media-controls.js"></script>	4 <script src="../media-controls.js"></script>

	5 <script src="../../resources/testharness.js"></script>

	6 <script src="../../resources/testharnessreport.js"></script>

10 <style>	7 <style>

11 video::cue(.red, .red2) { color:red }	8 video::cue(.red, .red2) { color:red }

12 video::cue(.green) { color:green }	9 video::cue(.green) { color:green }

13 </style>	10 </style>

	11 <div id='host'>

	12 </div>

14 <script>	13 <script>

15 var cueNode;	14 async_test(function(t) {

16 var seekedCount = 0;

17 var step = 0.4;

18 var initialTime = 0.6;

19 var endTime = 3.0

20

21 function seeked()

22 {

23 if (testEnded)

24 return;

25

26 cueNode = textTrackDisplayElement(video, 'cue').firstElementChild;

27 testExpected("getComputedStyle(cueNode).color", "rgb(255, 0, 0)", "!=");

28 cueNode = cueNode.nextElementSibling;

29 testExpected("getComputedStyle(cueNode).color", "rgb(0, 128, 0)", "!=");

30 cueNode = cueNode.nextElementSibling;

31 testExpected("getComputedStyle(cueNode).color", "rgb(255, 0, 0)", "!=");

32 endTest();

33 }

34

35 function loaded()

36 {

37 consoleWrite("Test that the cue is not styled when video is in a shadow tree and style is in a document.");

38 var host = document.getElementById('host');	15 var host = document.getElementById('host');

39 var shadowRoot = host.createShadowRoot();	16 var shadowRoot = host.createShadowRoot();

40 shadowRoot.innerHTML = '<video controls ><track src="captions-webvtt/styling -lifetime.vtt" kind="captions" default></video>';	17 shadowRoot.innerHTML = '<video controls ><track src="captions-webvtt/styling -lifetime.vtt" kind="captions" default></video>';

41 video = shadowRoot.querySelector('video');	18 var video = shadowRoot.querySelector('video');

42 video.src = findMediaFile('video', '../content/test');	19 video.src = findMediaFile('video', '../content/test');

43 video.id = "testvideo";	20 video.id = "testvideo";

44 waitForEvent('seeked', seeked);	21 video.onseeked = t.step_func_done(function(t) {
	mlamouri (slow - plz ping) 2016/04/06 09:22:58 I would prefer step_func() and call t.done() insid I would prefer step_func() and call t.done() inside because it is clearer. Srirama 2016/04/06 10:21:23 Hmm, philip suggested me to use step_func_done and Show quoted text On 2016/04/06 09:22:58, Mounir Lamouri wrote: > I would prefer step_func() and call t.done() inside because it is clearer. Hmm, philip suggested me to use step_func_done and remove t.done(), i think his intention is to keep the test code as compact and simple as possible. philipj_slow 2016/04/06 12:12:55 Yep, but I'm probably unusually devoted to minimal Show quoted text On 2016/04/06 10:21:23, Srirama wrote: > On 2016/04/06 09:22:58, Mounir Lamouri wrote: > > I would prefer step_func() and call t.done() inside because it is clearer. > > Hmm, philip suggested me to use step_func_done and remove t.done(), i think his > intention is to keep the test code as compact and simple as possible. Yep, but I'm probably unusually devoted to minimal tests, not surprised to see some disagreement. I'm pretty used to seeing step_func_done by now, but don't feel strongly about it. Mounir? mlamouri (slow - plz ping) 2016/04/07 13:37:17 Sure. Show quoted text On 2016/04/06 at 12:12:55, philipj wrote: > On 2016/04/06 10:21:23, Srirama wrote: > > On 2016/04/06 09:22:58, Mounir Lamouri wrote: > > > I would prefer step_func() and call t.done() inside because it is clearer. > > > > Hmm, philip suggested me to use step_func_done and remove t.done(), i think his > > intention is to keep the test code as compact and simple as possible. > > Yep, but I'm probably unusually devoted to minimal tests, not surprised to see some disagreement. I'm pretty used to seeing step_func_done by now, but don't feel strongly about it. Mounir? Sure.
45 waitForEventOnce('canplaythrough', function() { video.currentTime = initialT ime; });	22 var cueNode = textTrackDisplayElement(video, 'cue').firstElementChild;

46 }	23 assert_not_equals(getComputedStyle(cueNode).color, "rgb(255, 0, 0)");
	philipj_slow 2016/04/06 12:17:46 Hmm, the test was already like this, but this look Hmm, the test was already like this, but this looks a bit fragile, the actual color could change without the test failing. If the actual value is reliable, can you use that instead? Srirama 2016/04/07 13:46:18 Done. Show quoted text On 2016/04/06 12:17:46, philipj wrote: > Hmm, the test was already like this, but this looks a bit fragile, the actual > color could change without the test failing. If the actual value is reliable, > can you use that instead? Done.
	24 cueNode = cueNode.nextElementSibling;

	25 assert_not_equals(getComputedStyle(cueNode).color, "rgb(0, 128, 0)");

	26 cueNode = cueNode.nextElementSibling;

	27 assert_not_equals(getComputedStyle(cueNode).color, "rgb(255, 0, 0)");

	28 });

	29 video.oncanplaythrough = t.step_func(function(t) {
	philipj_slow 2016/04/06 12:17:46 I think you can not wait for canplaythrough and ju I think you can not wait for canplaythrough and just set currentTime directly. Originally this didn't work, but it should now, since a few years at least. Srirama 2016/04/07 13:46:18 Done. Show quoted text On 2016/04/06 12:17:46, philipj wrote: > I think you can not wait for canplaythrough and just set currentTime directly. > Originally this didn't work, but it should now, since a few years at least. Done.
	30 video.currentTime = 0.6;

	31 });

	32 });
	mlamouri (slow - plz ping) 2016/04/06 09:22:58 The test name goes here, not in <title>. ie. "Test The test name goes here, not in <title>. ie. "Test that the cue is not styled when video is in a shadow tree and style is in a document." Srirama 2016/04/06 10:21:24 Both are same, and having title instead of descrip Show quoted text On 2016/04/06 09:22:58, Mounir Lamouri wrote: > The test name goes here, not in <title>. > ie. "Test that the cue is not styled when video is in a shadow tree and style is > in a document." Both are same, and having title instead of description would make the test look more simple(even philipj suggested same thing). So what do you suggest? philipj_slow 2016/04/06 12:12:55 Right, when there's just a single test per file, I Show quoted text On 2016/04/06 10:21:24, Srirama wrote: > On 2016/04/06 09:22:58, Mounir Lamouri wrote: > > The test name goes here, not in <title>. > > ie. "Test that the cue is not styled when video is in a shadow tree and style > is > > in a document." > > Both are same, and having title instead of description would make the test look > more simple(even philipj suggested same thing). So what do you suggest? Right, when there's just a single test per file, I really like using <title>, because it's the first thing you see when reading the test. This style is very common in upstream web-platform-tests. mlamouri (slow - plz ping) 2016/04/07 13:37:17 What happens if the test fails? ie. what is printe Show quoted text On 2016/04/06 at 12:12:55, philipj wrote: > On 2016/04/06 10:21:24, Srirama wrote: > > On 2016/04/06 09:22:58, Mounir Lamouri wrote: > > > The test name goes here, not in <title>. > > > ie. "Test that the cue is not styled when video is in a shadow tree and style > > is > > > in a document." > > > > Both are same, and having title instead of description would make the test look > > more simple(even philipj suggested same thing). So what do you suggest? > > Right, when there's just a single test per file, I really like using <title>, because it's the first thing you see when reading the test. This style is very common in upstream web-platform-tests. What happens if the test fails? ie. what is printed? philipj_slow 2016/04/07 13:40:32 It's the same as if the title is given as an argum Show quoted text On 2016/04/07 13:37:17, Mounir Lamouri wrote: > On 2016/04/06 at 12:12:55, philipj wrote: > > On 2016/04/06 10:21:24, Srirama wrote: > > > On 2016/04/06 09:22:58, Mounir Lamouri wrote: > > > > The test name goes here, not in <title>. > > > > ie. "Test that the cue is not styled when video is in a shadow tree and > style > > > is > > > > in a document." > > > > > > Both are same, and having title instead of description would make the test > look > > > more simple(even philipj suggested same thing). So what do you suggest? > > > > Right, when there's just a single test per file, I really like using <title>, > because it's the first thing you see when reading the test. This style is very > common in upstream web-platform-tests. > > What happens if the test fails? ie. what is printed? It's the same as if the title is given as an argument to async_test. The title isn't changed by the test failing, just the things that are printed to the right of the title. Results or whatever it may be labeled. Srirama 2016/04/07 13:46:18 yes, this is the sample output for your reference. Show quoted text On 2016/04/07 13:40:32, philipj wrote: > On 2016/04/07 13:37:17, Mounir Lamouri wrote: > > On 2016/04/06 at 12:12:55, philipj wrote: > > > On 2016/04/06 10:21:24, Srirama wrote: > > > > On 2016/04/06 09:22:58, Mounir Lamouri wrote: > > > > > The test name goes here, not in <title>. > > > > > ie. "Test that the cue is not styled when video is in a shadow tree and > > style > > > > is > > > > > in a document." > > > > > > > > Both are same, and having title instead of description would make the test > > look > > > > more simple(even philipj suggested same thing). So what do you suggest? > > > > > > Right, when there's just a single test per file, I really like using > <title>, > > because it's the first thing you see when reading the test. This style is very > > common in upstream web-platform-tests. > > > > What happens if the test fails? ie. what is printed? > > It's the same as if the title is given as an argument to async_test. The title > isn't changed by the test failing, just the things that are printed to the right > of the title. Results or whatever it may be labeled. yes, this is the sample output for your reference. This is a testharness.js-based test. FAIL Test that the cue is not styled when video is in a shadow tree and style is in a document. assert_equals: expected "rgb(255, 255, 0)" but got "rgb(255, 255, 255)" Harness: the test ran to completion.
47 </script>	33 </script>

48 </head>

49 <body onload="loaded()">

50 <div id='host'>

51 </div>

52 </body>

53 </html>

OLD	NEW