third_party/WebKit/LayoutTests/media/video-controls-captions.html - Issue 2121613002: Convert video-controls-[captions, overlay, track]* tests to testharness.js

Side by Side Diff: third_party/WebKit/LayoutTests/media/video-controls-captions.html

Issue 2121613002: Convert video-controls-[captions, overlay, track]* tests to testharness.js (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: fix Created 4 years, 5 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 | « third_party/WebKit/LayoutTests/media/media-controls.js ('k') | third_party/WebKit/LayoutTests/media/video-controls-captions-expected.txt » ('j') | third_party/WebKit/LayoutTests/media/video-controls-captions-on-off.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>Tests that the closed captions button enables track switching.</title>

3 <head>	3 <script src="../resources/testharness.js"></script>

4 <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />	4 <script src="../resources/testharnessreport.js"></script>

5 <title>Test closed caption track selection functionality.</title>	5 <script src="media-file.js"></script>

6 <script src=media-file.js></script>	6 <script src="media-controls.js"></script>

7 <script src=media-controls.js></script>	7 <video controls>

8 <!-- TODO(foolip): Convert test to testharness.js. crbug.com/588956	8 <track src="track/captions-webvtt/captions-fast.vtt" kind="captions">

9 (Please avoid writing new tests using video-test.js) -->	9 </video>

10 <script src=video-test.js></script>	10 <script>

11 <script>	11 async_test(function(t) {

12 var track;	12 var video = document.querySelector("video");

	13 var track = document.querySelector("track");
	fs 2016/07/04 08:46:55 We could move this into the relevant scope, right? We could move this into the relevant scope, right? (And make 'track' a local elsewhere.) Srirama 2016/07/04 13:00:23 Done. Show quoted text On 2016/07/04 08:46:55, fs wrote: > We could move this into the relevant scope, right? (And make 'track' a local > elsewhere.) Done.
13	14

14 function addTextTrackThroughJS()	15 video.oncanplaythrough = t.step_func(function() {

15 {	16 assert_true(isClosedCaptionsButtonVisible(video));

16 consoleWrite("");

17 consoleWrite(" Add a text track through JS to the video element ");

18 var t = video.addTextTrack('captions', 'English', 'en');

19 t.addCue(new VTTCue(0.0, 10.0, 'Some random caption text'));

20 }

21	17

22 function addUnloadableHTMLTrackElement()	18 // The captions track should be listed in textTracks, but not yet loaded .

23 {	19 assert_equals(video.textTracks.length, 1);

24 consoleWrite("");	20 assert_equals(video.textTracks[0].mode, "disabled");

25 consoleWrite(" Add non-default text track through HTML with unload able URI ");	21 assert_throws(null, function() {
	Srirama 2016/07/04 07:56:56 This is for "text track container" error and the o This is for "text track container" error and the one below is for "cue" error. So should i compare the error message to differentiate or is there a better way? fs 2016/07/04 08:46:55 I think the easiest way forward is just to add an Show quoted text On 2016/07/04 at 07:56:56, Srirama wrote: > This is for "text track container" error and the one below is for "cue" error. So should i compare the error message to differentiate or is there a better way? I think the easiest way forward is just to add an "annotation" (description) to the assertions, and describe what they are for. Better would probably be to break up textTrackDisplayElement into several usable pieces whose result could be asserted (it's very "intelligent" as it is...) foolip 2016/07/04 08:49:31 It's not always clear what checkCaptionsDisplay is Show quoted text On 2016/07/04 07:56:56, Srirama wrote: > This is for "text track container" error and the one below is for "cue" error. > So should i compare the error message to differentiate or is there a better way? It's not always clear what checkCaptionsDisplay is hoping to check, but without the error messages in -expected.txt this is indeed testing less than before. textTrackDisplayElement is doing a lot, this might all look simpler if it's split into helpers: textTrackContainerElement textTrackCueElementById textTrackCueElementAtIndex Then here just use assert_equals(textTrackContainerElement(video), null) and below maybe textTrackCueElementAtIndex(0) if that returns null if out of range? Or if the only interesting this is to check if something is rendered or not, which seems to be the case, write a helper that just checks if the bounding client rect of the container is zero or not. (Not tested.) Srirama 2016/07/04 13:00:23 Attempted the first approach. The helpers are a bi Show quoted text On 2016/07/04 08:49:31, Philip Jägenstedt wrote: > On 2016/07/04 07:56:56, Srirama wrote: > > This is for "text track container" error and the one below is for "cue" error. > > So should i compare the error message to differentiate or is there a better > way? > > It's not always clear what checkCaptionsDisplay is hoping to check, but without > the error messages in -expected.txt this is indeed testing less than before. > textTrackDisplayElement is doing a lot, this might all look simpler if it's > split into helpers: > > textTrackContainerElement > textTrackCueElementById > textTrackCueElementAtIndex > > Then here just use assert_equals(textTrackContainerElement(video), null) and > below maybe textTrackCueElementAtIndex(0) if that returns null if out of range? > > Or if the only interesting this is to check if something is rendered or not, > which seems to be the case, write a helper that just checks if the bounding > client rect of the container is zero or not. (Not tested.) Attempted the first approach. The helpers are a bit interdependent right now, or should i make them independent with each taking "video" as parameter?
	22 textTrackDisplayElement(video, "display");

	23 });

26	24

27 track = document.createElement("track");	25 track.onload = t.step_func(function() {

28 track.setAttribute("kind", "captions");	26 assert_equals(textTrackDisplayElement(video, "display").innerText, " Lorem");

29 track.setAttribute("srclang", "en");

30 track.setAttribute("src", "invalid.vtt");

31	27

32 track.addEventListener("error", trackError);	28 // Captions should not be visible after Off is clicked.

	29 turnClosedCaptionsOff(video);

	30 assert_throws(null, function() {

	31 textTrackDisplayElement(video, "display");

	32 });

33	33

34 video.appendChild(track);	34 // Remove DOM node representing the track element.

35 testExpected("track.readyState", HTMLTrackElement.NONE);	35 video.removeChild(track);
	foolip 2016/07/04 08:49:32 Just track.remove() works too. Just track.remove() works too. Srirama 2016/07/04 13:00:23 Done. Show quoted text On 2016/07/04 08:49:32, Philip Jägenstedt wrote: > Just track.remove() works too. Done.
36 testExpected("track.track.mode", "disabled");	36 assert_false(isClosedCaptionsButtonVisible(video));

37 testExpected("video.textTracks.length", 1);

38 }

39

40 function removeHTMLTrackElement()

41 {

42 consoleWrite("");

43 consoleWrite(" Remove DOM node representing the track element ") ;

44 var htmlTrack = video.children[0];

45 video.removeChild(htmlTrack);

46 }

47

48 function checkCaptionsDisplay()

49 {

50 // When no tracks are loaded, this should report no text track conta iner,

51 // when tracks are loaded but not visible, should report no cues vis ible,

52 // when tracks are loaded and visible, should properly check the tex t.

53 testExpected("textTrackDisplayElement(video, 'display').innerText", "Lorem");

54 }

55

56 function startTest()

57 {

58 if (!window.eventSender) {

59 consoleWrite("No eventSender found.");

60 failTest();

61 }

62

63 findMediaElement();

64 testClosedCaptionsButtonVisibility(true);

65

66 consoleWrite("");

67 consoleWrite(" The captions track should be listed in textTracks, but not yet loaded. ");

68 testExpected("video.textTracks.length", 1);

69 testExpected("video.textTracks[0].mode", "disabled");

70 checkCaptionsDisplay();

71

72 consoleWrite("");

73 consoleWrite(" Captions track should load and captions should beco me visible after a track is selected ");

74

75 // Note: the test flow continues with "testCCTrackSelectionFunctiona lity" when the

76 // "load" event of the single TextTrack fires up. While the test str ucture

77 // might seem weird, this avoids timeouts.

78 selectTextTrack(video, 0);

79 }

80

81 function testCCTrackSelectionFunctionality()

82 {

83 checkCaptionsDisplay();

84

85 consoleWrite("");

86 consoleWrite(" Captions should not be visible after Off is clicked ");

87 turnClosedCaptionsOff(video);

88 checkCaptionsDisplay();

89

90 removeHTMLTrackElement();

91 testClosedCaptionsButtonVisibility(false);

92	37

93 addUnloadableHTMLTrackElement();	38 addUnloadableHTMLTrackElement();

94 testClosedCaptionsButtonVisibility(true);	39 assert_true(isClosedCaptionsButtonVisible(video));

95	40

96 consoleWrite("");	41 selectTextTrackAtIndex(video, 0);
	foolip 2016/07/04 08:49:31 Since you're renaming it anyway, I think clickText Since you're renaming it anyway, I think clickTextTrackAtIndex would be even better, so that it's clear this isn't using the DOM-exposed APIs to enable the track. Srirama 2016/07/04 13:00:23 Done. Show quoted text On 2016/07/04 08:49:31, Philip Jägenstedt wrote: > Since you're renaming it anyway, I think clickTextTrackAtIndex would be even > better, so that it's clear this isn't using the DOM-exposed APIs to enable the > track. Done.
97 selectTextTrack(video, 0);	42 });

98 }

99	43

100 function trackError()	44 // Captions track should load and captions should become visible after a track is selected.

101 {	45 selectTextTrackAtIndex(video, 0);

102 consoleWrite(" Track failed to load ");	46 });

103 testClosedCaptionsButtonVisibility(false);

104	47

105 addTextTrackThroughJS();	48 function addUnloadableHTMLTrackElement() {

106 testClosedCaptionsButtonVisibility(true);	49 // Add non-default text track through HTML with unloadable URI.

	50 track = document.createElement("track");
	fs 2016/07/04 08:46:55 Make 'track' local (per above.) Make 'track' local (per above.) Srirama 2016/07/04 13:00:23 Done. Show quoted text On 2016/07/04 08:46:55, fs wrote: > Make 'track' local (per above.) Done.
	51 track.setAttribute("kind", "captions");

	52 track.setAttribute("srclang", "en");

	53 track.setAttribute("src", "invalid.vtt");

107	54

108 endTest();	55 track.onerror = t.step_func_done(function() {

109 }	56 // Track failed to load.

	57 assert_false(isClosedCaptionsButtonVisible(video));

	58 // Add a text track through JS to the video element.

	59 var newTrack = video.addTextTrack("captions", "English", "en");

	60 newTrack.addCue(new VTTCue(0, 10, "Some random caption text"));
	foolip 2016/07/04 08:49:31 Is this line really needed for the test to pass? T Is this line really needed for the test to pass? The number of cues aren't part of the logic for text track UI I think. Srirama 2016/07/04 13:00:23 Done. Show quoted text On 2016/07/04 08:49:31, Philip Jägenstedt wrote: > Is this line really needed for the test to pass? The number of cues aren't part > of the logic for text track UI I think. Done.
	61 assert_true(isClosedCaptionsButtonVisible(video));

	62 });

110	63

111 function loaded()	64 video.appendChild(track);

112 {	65 assert_equals(track.readyState, HTMLTrackElement.NONE);

113 findMediaElement();	66 assert_equals(track.track.mode, "disabled");

114 waitForEvent('canplaythrough', startTest);	67 assert_equals(video.textTracks.length, 1);

	68 }

115	69

116 video.src = findMediaFile('video', 'content/counting');	70 video.src = findMediaFile("video", "content/counting");

117 }	71 });

118 </script>	72 </script>

119 </head>

120 <body onload="loaded()">

121 <p>Tests that the closed captions button enables track switching</p>

122 <video controls>

123 <track src="track/captions-webvtt/captions-fast.vtt" kind="captions" onl oad="testCCTrackSelectionFunctionality()">

124 </video>

125 </body>

126 </html>

OLD	NEW