Skip to content

Commit

Permalink
test: Fix test flake on Safari waiting for ended event
Browse files Browse the repository at this point in the history
The test "StreamingEngine VOD plays" would fail ~10% of the time on
Safari 14 with "Timeout waiting for event ended".  When this happened,
I found that the "ended" flag was set, and currentTime exceeded
duration, but the "ended" event had not fired.

This changes the waitForEnd() method in our tests to be more flexible,
and to tolerate an "ended" event being missed.  It will now consider
media to be "ended" on the "ended" event, or when the "ended" flag is
seen, or when currentTime equals or exceeds duration.

With this change, the tests passed 50 times in a row.

Change-Id: I6ce7dcc38e6ed43c73bfbd91ef96957df46cc70b
  • Loading branch information
joeyparrish committed May 10, 2021
1 parent 24d10ed commit f3ef38b
Showing 1 changed file with 36 additions and 2 deletions.
38 changes: 36 additions & 2 deletions test/test/util/waiter.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,43 @@ shaka.test.Waiter = class {
* @return {!Promise}
*/
waitForEnd(mediaElement) {
if (mediaElement.ended) {
// Sometimes, the ended flag is not set, or the event does not fire,
// (I'm looking at **you**, Safari), so also check if we've reached the
// duration.
if (mediaElement.ended ||
mediaElement.currentTime >= mediaElement.duration) {
return Promise.resolve();
}
return this.waitForEvent(mediaElement, 'ended');

// The name of what we're waiting for.
const goalName = 'end of media';

// Cleanup on timeout.
const cleanup = () => {
this.eventManager_.unlisten(mediaElement, 'timeupdate');
this.eventManager_.unlisten(mediaElement, 'ended');
};

// The conditions for success. Don't rely on either time, or the ended
// flag, or the ended event, specifically. Any of these is sufficient.
// This flexibility cuts down on test flake on Safari (currently 14) in
// particular, where the flag might be set, but the ended event did not
// fire.
const p = new Promise((resolve) => {
this.eventManager_.listen(mediaElement, 'timeupdate', () => {
if (mediaElement.currentTime >= mediaElement.duration ||
mediaElement.ended) {
cleanup();
resolve();
}
});
this.eventManager_.listen(mediaElement, 'ended', () => {
cleanup();
resolve();
});
});

return this.waitUntilGeneric_(goalName, p, cleanup, mediaElement);
}

/**
Expand Down Expand Up @@ -256,6 +289,7 @@ shaka.test.Waiter = class {
'ready state', mediaElement.readyState,
'playback rate', mediaElement.playbackRate,
'paused', mediaElement.paused,
'ended', mediaElement.ended,
'buffered', buffered);
}
};

0 comments on commit f3ef38b

Please sign in to comment.