MediaRecorder tests for Pause functionality

content/test/data/media/mediarecorder_test.html

Index: content/test/data/media/mediarecorder_test.html
diff --git a/content/test/data/media/mediarecorder_test.html b/content/test/data/media/mediarecorder_test.html
index 53566e9e512791263d7b1b7a132d3aaafed01f22..e6e2125503c8ae6d019eb21122807620c2e11968 100644
--- a/content/test/data/media/mediarecorder_test.html
+++ b/content/test/data/media/mediarecorder_test.html
@@ -21,7 +21,6 @@ const DEFAULT_TIME_SLICE = 100;
 
 function createAndStartMediaRecorder(stream, mimeType) {
   return new Promise(function(resolve, reject) {
-    console.log('Starting MediaRecorder instance');
     document.getElementById("video").src = URL.createObjectURL(stream);
     var recorder = new MediaRecorder(stream, mimeType);
     console.log('Recorder object created.');
@@ -32,7 +31,6 @@ function createAndStartMediaRecorder(stream, mimeType) {
 
 function createAndStartWithTimeSliceMediaRecorder(stream, mimeType, slice) {

[mcasas, 2015/12/08 02:49:05]

Code duplication???
Please merge this function with the previous.

[cpaulin (no longer in chrome), 2015/12/08 18:29:11]

Miguel,
Patrik explicitly wanted it separate, so there you go. At first I had it
combined into one.

[mcasas, 2015/12/11 00:08:56]

The only really different line is 37 and you could
just quickly check if the |slice| parameter is
there by |slice !== 'undefined'| (note that 
|slice == 0| is not the same as this function
being called with no |slice|.

I think these 3 methods can be collapsed since they
are essentially the same. What about
createMediaRecorderAndStart(...) ?

   return new Promise(function(resolve, reject) {
-    console.log('Starting MediaRecorder instance');
     document.getElementById("video").src = URL.createObjectURL(stream);
     var recorder = new MediaRecorder(stream, mimeType);
     console.log('Recorder object created.');
@@ -44,7 +42,6 @@ function createAndStartWithTimeSliceMediaRecorder(stream, mimeType, slice) {
 
 function createMediaRecorder(stream, mimeType) {
   return new Promise(function(resolve, reject) {
-    console.log('Starting MediaRecorder instance');
     document.getElementById("video").src = URL.createObjectURL(stream);
     var recorder = new MediaRecorder(stream, mimeType);
     console.log('Recorder object created.');
@@ -53,7 +50,6 @@ function createMediaRecorder(stream, mimeType) {
 }
 
 function testStartAndRecorderState() {
-  console.log('testStartAndRecorderState started.');
   navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
       .then(function(stream) {
         return createAndStartMediaRecorder(stream,
@@ -71,7 +67,6 @@ function testStartAndRecorderState() {
 }
 
 function testStartStopAndRecorderState() {
-  console.log('testStartStopAndRecorderState started.');
   var theRecorder;
   navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
       .then(function(stream) {
@@ -94,7 +89,6 @@ function testStartStopAndRecorderState() {
 }
 
 function testStartAndDataAvailable() {
-  console.log('testStartAndDataAvailable started.');
   var videoSize = 0;
   var emptyBlobs = 0;
   var timeStamps = [];
@@ -132,7 +126,6 @@ function testStartAndDataAvailable() {
 }
 
 function testStartWithTimeSlice() {
-  console.log('testStartWithTimeSlice started.');
   var videoSize = 0;
   var emptyBlobs = 0;
   var timeStamps = [];
@@ -156,16 +149,6 @@ function testStartWithTimeSlice() {
               return videoSize > 0 && timeStamps.length > 10;
             });
       })
-      .then(function() {
-        var timeDeltas = getTimeStampDeltas(timeStamps);
-        timeDeltas.forEach(function(timeDelta) {
-          assertTrue(timeDelta >= DEFAULT_TIME_SLICE, 'A time slice was ' +
-              timeDelta + ' ms which is shorter than the expected minimum '
-              + DEFAULT_TIME_SLICE + 'ms');
-          });
-        assertTrue(emptyBlobs == 0, 'Recording has ' + emptyBlobs +
-            ' empty blobs, there should be no such empty blobs.');
-      })

[mcasas, 2015/12/11 00:08:57]

Why is this part gone?

[cpaulin (no longer in chrome), 2015/12/11 19:56:55]

It is a bit flaky, sometimes the time stamps are a little bit late and then it
fails. It was a nice to have thing, I am not sure it is worth the added
flakiness.

       .catch(function(err) {
         return failTest(err.toString());
       })
@@ -175,7 +158,6 @@ function testStartWithTimeSlice() {
 }
 
 function testStartAndStartEventTriggered() {
-  console.log('testStartAndStartEventTriggered started.');
   var theRecorder;
   var startEventReceived = false;
   navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
@@ -206,7 +188,6 @@ function testStartAndStartEventTriggered() {
 }
 
 function testStartStopAndStopEventTriggered() {
-  console.log('testStartStopAndStopEventTriggered started.');
   var theRecorder;
   var stopEventReceived = false;
   navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
@@ -235,7 +216,6 @@ function testStartStopAndStopEventTriggered() {
 
 function testResumeAndRecorderState() {
   var theRecorder;
-  console.log('testResumeAndRecorderState started.');
   navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
       .then(function(stream) {
         return createAndStartMediaRecorder(stream,
@@ -262,7 +242,6 @@ function testResumeAndRecorderState() {
 function testResumeAndResumeEventTriggered() {
   var theRecorder;
   var resumeEventReceived = false;
-  console.log('testStartAndResumeEventTriggered started.');
   navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
       .then(function(stream) {
         return createAndStartMediaRecorder(stream,
@@ -293,7 +272,6 @@ function testResumeAndResumeEventTriggered() {
 }
 
 function testNoResumeWhileRecorderInactive() {
-  console.log('testNoResumeWhileRecorderInactive started.');
   navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
       .then(function(stream) {
         return createMediaRecorder(stream, DEFAULT_RECORDER_MIME_TYPE);
@@ -301,16 +279,15 @@ function testNoResumeWhileRecorderInactive() {
       .then(function(recorder) {
         recorder.resume();
       })
-      .catch(function(err) {
-        reportTestSuccess();
-      })
       .then(function() {
         return failTest('Recorder resumed recording from inactive state.');
+      })
+      .catch(function(err) {
+        reportTestSuccess();
       });
 }
 
 function testResumeAndDataAvailable() {
-  console.log('testResumeAndDataAvailable started.');
   var theRecorder;
   var videoSize = 0;
   var emptyBlobs = 0;
@@ -355,6 +332,131 @@ function testResumeAndDataAvailable() {
       });
 }
 
+function testPauseAndRecorderState() {
+  var theRecorder;
+  navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
+      .then(function(stream) {
+        return createAndStartMediaRecorder(stream,
+            DEFAULT_RECORDER_MIME_TYPE);
+      })
+      .then(function(recorder) {
+        theRecorder = recorder;
+        theRecorder.pause();
+      })
+      .then(function() {
+        assertEquals('paused', theRecorder.state);
+      })
+      .catch(function(err) {
+        return failTest(err.toString());
+      })
+      .then(function() {
+        reportTestSuccess();
+      });
+}
+
+function testPauseStopAndRecorderState() {
+  var theRecorder;
+  navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
+      .then(function(stream) {
+        return createAndStartMediaRecorder(stream,
+            DEFAULT_RECORDER_MIME_TYPE);
+      })
+      .then(function(recorder) {
+        theRecorder = recorder;
+        theRecorder.pause();
+      })
+      .then(function() {
+        theRecorder.stop();
+      })
+      .then(function() {
+        assertEquals('inactive', theRecorder.state);
+      })
+      .catch(function(err) {
+        return failTest(err.toString());
+      })
+      .then(function() {
+        reportTestSuccess();
+      });
+}
+
+function testPauseAndPauseEventTriggered() {

[mcasas, 2015/12/11 00:08:57]

What about merging testPauseAndPauseEventTriggered()
with testPauseAndRecorderState()?
I understand the lecture about each test should
trigger one thing only, but in particular 
checking for a particular state and grabbing an
Event are sequential and, with the appropriate
checking, easily distinguishable in an error log.

This also applies to e.g. testStartAndRecorderState()
and testStartStopAndStopEventTriggered(). 

[cpaulin (no longer in chrome), 2015/12/11 19:56:55]

You are preaching to the convert... I am not sure I will go for it since it
might mean me changing this 2 more times ;-(

+  var pauseEventReceived = false;
+  navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
+      .then(function(stream) {
+        return createAndStartMediaRecorder(stream,
+            DEFAULT_RECORDER_MIME_TYPE);
+      })
+      .then(function(recorder) {
+        recorder.onpause = function(event) {
+          pauseEventReceived = true;
+        }
+        recorder.pause();
+      })
+      .then(function() {
+        return waitFor('Making sure the pause event has been received',
+            function() {
+              return pauseEventReceived == true;
+            });
+      })
+      .catch(function(err) {
+        return failTest(err.toString());
+      })
+      .then(function() {
+        reportTestSuccess();
+      });
+}
+
+function testPauseAndNoDataAvailable() {
+  var theRecorder;
+  var videoSize = 0;
+  var emptyBlobs = 0;
+  navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
+      .then(function(stream) {
+        return createAndStartMediaRecorder(stream,
+            DEFAULT_RECORDER_MIME_TYPE);
+      })
+      .then(function(recorder) {
+        theRecorder = recorder;
+        theRecorder.pause();
+      })
+      .then(function() {
+        theRecorder.ondataavailable = function(event) {
+          if (event.data.size > 0) {
+            videoSize  += event.data.size;
+          } else {
+            emptyBlobs += 1;
+          }
+        };
+      })
+      .then(function() {
+        return waitFor('Make sure no dataavailable events are received',
+            function() {
+              return videoSize == 0 && emptyBlobs == 0;
+            });
+      })
+      .catch(function(err) {
+        return failTest(err.toString());
+      })
+      .then(function() {
+        reportTestSuccess();
+      });
+}
+
+function testNoPauseWhileRecorderInactive() {
+  navigator.mediaDevices.getUserMedia(DEFAULT_CONSTRAINTS)
+      .then(function(stream) {
+        return createMediaRecorder(stream, DEFAULT_RECORDER_MIME_TYPE);
+      })
+      .then(function(recorder) {
+        recorder.pause();
+      })
+      .then(function() {
+        return failTest('Recorder paused recording from inactive state.');
+      })
+      .catch(function(err) {
+        reportTestSuccess();

[mcasas, 2015/12/11 00:08:56]

In some tests there is a catch() ending in 
reportTestSuccess() and that's somehow
surprising. WebKit has assert_throws() for
this, to make it very explicit [1]. Why
not copy the appropriate parts (i.e. until
|if (code == null) ... return;|) in our
test utilities js? (Also applies to l.286,
at least).

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[cpaulin (no longer in chrome), 2015/12/11 19:56:55]

Because catch is a nice feature of promise that really works well here. And this
is a negative test: we want it to fail.

+      });
+}
 
 </script>
 </body>