Don’t delete that yet, but we will end up removing it when it’s no longer used.
From here what should I do next? https://jsfiddle.net/x3s0ehd1/
fdescribe("playerReady", function() {
let video;
let afterPlayerReadySpy;
function triggerOnReady() {
const onReady = options.events.onReady;
const evt = {
target: player
};
onReady(evt);
}
function initVideoPlayer() {
const spy = jasmine.createSpy("afterPlayerReady-handler");
initAfterPlayerReady(spy);
return spy;
}
beforeEach(function() {
video = createVideo();
});
it("sets volume", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
videoPlayer.initVideoPlayer(video, playlist);
//when
triggerOnReady();
//then
expect(player.setVolume).toHaveBeenCalled();
});
it("dispatches the afterPlayerReady event", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
videoPlayer.initVideoPlayer(video, playlist);
//when
triggerOnReady();
//then
expect(afterPlayerReadySpy).toHaveBeenCalled();
});
it("enables setShuffle", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
videoPlayer.initVideoPlayer(video, playlist);
//when
triggerOnReady();
//then
expect(player.setShuffle).toHaveBeenCalledWith(true);
});
it("resets play order", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
videoPlayer.initVideoPlayer(video, playlist);
//when
triggerOnReady();
//then
expect(player.playVideoAt).toHaveBeenCalledWith(0);
});
it("stops video after resetting play order", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
videoPlayer.initVideoPlayer(video, playlist);
//when
triggerOnReady();
//then
expect(player.stopVideo).toHaveBeenCalled();
});
});
});
Well the error that’s currently shown is:
TypeError: videoPlayer.addPlayer is not a function
That’s happening in the onYoutubeIframeAPIReady() function.
function onYouTubeIframeAPIReady() {
const cover = document.querySelector(".play");
const wrapper = cover.parentElement;
const frameContainer = wrapper.querySelector(".video");
videoPlayer.addPlayer(frameContainer, playlist);
}
The videoPlayer.addPlayer() function call is attempting to use the external access to addPlayer that has just been removed.
Instead of using external access, we can just use internal access by replacing videoPlayer.addPlayer with just addPlayer
1 Like
Like this: https://jsfiddle.net/r4hcbmjz/
It still says 5 failures.
function onYouTubeIframeAPIReady() {
const cover = document.querySelector(".play");
const wrapper = cover.parentElement;
const frameContainer = wrapper.querySelector(".video");
addPlayer(frameContainer, playlist);
}
Yes, but they are now a different type of failure, which is progress.
The failure now is TypeError: videoPlayer.initVideoPlayer is not a function
Here is what the test looks like:
it("stops video after resetting play order", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
videoPlayer.initVideoPlayer(video, playlist);
It is the videoPlayer.initVideoPlayer line that is the problem there. You need to completely remove that line.
I am down to 8 failures. https://jsfiddle.net/gxo9fkL3/1/
They all say:
TypeError: videoPlayer.addPlayer is not a function
How do I fix all of those?
And tis one:
Expected function to throw an exception with message ‘A playlist is required.’, but it threw an exception with message ‘videoPlayer.addPlayer is not a function’.
There are several issues in the addPlayer section of tests, so let’s use fdescribe instead of describe, to force only those tests to run.
// describe("addPlayer", function() {
fdescribe("addPlayer", function() {
Here’s one of the error messages:
TypeError: videoPlayer.addPlayer is not a function
And here’s the test in which that error occurs:
it("it has playerVars", function() {
//given
player = undefined;
//when
videoPlayer.addPlayer(video, playlist);
//then
const playerVars = options.playerVars;
expect(typeof playerVars.cc_load_policy).toBe("number");
expect(playerVars.cc_load_policy).toBe(0);
});
- The text of “it has playerVars” shouldn’t start with it, it should just say “has playerVars” instead.
- I don’t know why you are setting player to be undefined. so remove that.
It is the options variable that thethensection is accessing, so where player was, set options to be undefined instead. - The addPlayer line does nothing so remove that, and replace it with a call to videoPlayer.init() using videoIds as the argument given to that call.
That test then passes, with suitable information in the given, when, and then sections of the test.
1 Like
3 failures are now left: https://jsfiddle.net/x2dqs6tk/
describe("videoPlayer tests", function() {
let iframe;
function checkIframeScript() {
const firstScript = document.querySelectorAll("script")[0];
const url = firstScript.src;
if (!url.includes("example.com")) {
throw new Error("Expected script to have example.com but found " + url);
}
}
beforeAll(function() {
iframe = stubYT();
videoPlayer.originalInit = videoPlayer.init;
videoPlayer.init = function testInit(videoids) {
videoPlayer.setYoutubeIframeUrl("https://www.example.com/iframe_api");
videoPlayer.originalInit(videoids);
checkIframeScript();
window.onYouTubeIframeAPIReady();
};
});
afterAll(function() {
videoPlayer.init = videoPlayer.originalInit;
});
let options;
let player;
let playlist = "0dgNc5S8cLI,mnfmQe8Mv1g,CHahce95B1g,2VwsvrPFr9w";
const videoIds = ["0dgNc5S8cLI", "CHahce95B1g", "mnfmQe8Mv1g", "2VwsvrPFr9w"];
function createVideo() {
const video = document.createElement("div");
video.classList.add("video");
return video;
}
function stubYT() {
const randomPropertyNames = ["d", "e", "f", "g", "h"].sort(function() {
return Math.random() - 0.5;
});
const iframe = document.createElement("iframe");
window.YT = {
Player: function makePlayer(video, playerOptions) {
options = playerOptions;
player = {
i: {
h: playerOptions
},
m: video,
playVideoAt: jasmine.createSpy("playVideoAt-handler"),
setShuffle: jasmine.createSpy("setShuffle-handler"),
setVolume: jasmine.createSpy("setVolume-handler"),
stopVideo: jasmine.createSpy("stopVideo-handler")
};
player[randomPropertyNames[0]] = iframe;
return player;
}
};
return iframe;
}
function initAfterPlayerReady(spy) {
videoPlayer.afterPlayerReady = spy;
videoPlayer.init(videoIds);
}
function triggerAfterPlayerReady(el) {
const afterPlayerReady = new window.CustomEvent("afterPlayerReady");
el.dispatchEvent(afterPlayerReady);
}
describe("init", function() {
it("makes onYouTubeIframeAPIReady available", function() {
videoPlayer.init(videoIds);
expect(typeof window.onYouTubeIframeAPIReady).toBe("function");
});
it("loads iframe script", function() {
//given
//when
videoPlayer.init(videoIds);
//then
const src = document.querySelector("script").src;
expect(src).toContain("iframe_api");
});
it("init with separate items in a list of videos for the playlist", function() {
//given
const videoIds = ["0dgNc5S8cLI", "mnfmQe8Mv1g", "CHahce95B1g", "2VwsvrPFr9w"];
//when
videoPlayer.init(videoIds);
//then
const playlist = options.playerVars.playlist;
expect(playlist).toBe("0dgNc5S8cLI,mnfmQe8Mv1g,CHahce95B1g,2VwsvrPFr9w");
});
it("init with another list of videos for the playlist", function() {
//given
const videoIds = ["0dgNc5S8cLI", "CHahce95B1g", "mnfmQe8Mv1g", "2VwsvrPFr9w"];
//when
videoPlayer.init(videoIds);
//then
const playlist = options.playerVars.playlist;
expect(playlist).toBe("0dgNc5S8cLI,CHahce95B1g,mnfmQe8Mv1g,2VwsvrPFr9w");
});
it("afterPlayerReady handler", function() {
//given
const spy = jasmine.createSpy("afterPlayerReady-handler");
initAfterPlayerReady(spy);
const video = createVideo();
//videoPlayer.addPlayer(video, playlist);
videoPlayer.init(videoIds, playlist);
//when
triggerAfterPlayerReady(iframe);
//then
expect(spy).toHaveBeenCalled();
});
});
describe("addPlayer", function() {
let video;
beforeEach(function() {
video = createVideo();
});
it("addPlayer requires a video element", function() {
//given
const badVideo = document.createElement("div");
//then
function badArgument() {
//videoPlayer.addPlayer(badVideo, playlist);
videoPlayer.init(badVideo, playlist);
}
expect(badArgument).toThrowError(TypeError, /Element needs a video classname/);
});
it("passes video to the player object", function() {
//given
options = undefined;
//when
videoPlayer.init(videoIds, playlist);
//then
expect(player.m.classList).toContain("video");
});
it(" has dimensions", function() {
//given
options = undefined;
//when
//videoPlayer.addPlayer(video, playlist);
videoPlayer.init(videoIds, playlist);
//then
expect(typeof options.width).toBe("number");
expect(options.width).toBeGreaterThan(0);
});
it("has playerVars", function() {
//given
options = undefined;
//when
videoPlayer.init(videoIds, playlist);
//then
const playerVars = options.playerVars;
expect(typeof playerVars.cc_load_policy).toBe("number");
expect(playerVars.cc_load_policy).toBe(0);
});
it("needs a playlist", function() {
//given
const playlist = null;
//when
const addPlayerWithNoPlaylist = function() {
//videoPlayer.addPlayer(video, playlist);
videoPlayer.init(videoIds, playlist);
};
//then
expect(addPlayerWithNoPlaylist).toThrowError("A playlist is required.");
});
it("has a playlist", function() {
//given
options = undefined;
const playlist = ["0dgNc5S8cLI", "mnfmQe8Mv1g"];
//when
//videoPlayer.addPlayer(video, playlist);
videoPlayer.init(videoIds, playlist);
//then
const playerVars = options.playerVars;
expect(playerVars.playlist).toBe(playlist);
});
it("has onReady event", function() {
//given
options = undefined;
//when
//videoPlayer.addPlayer(video, playlist);
videoPlayer.init(videoIds, playlist);
//then
expect(typeof options.events.onReady).toBe("function");
});
});
describe("playerReady", function() {
let video;
let afterPlayerReadySpy;
function triggerOnReady() {
const onReady = options.events.onReady;
const evt = {
target: player
};
onReady(evt);
}
function initVideoPlayer() {
const spy = jasmine.createSpy("afterPlayerReady-handler");
initAfterPlayerReady(spy);
return spy;
}
beforeEach(function() {
video = createVideo();
});
it("sets volume", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
//when
triggerOnReady();
//then
expect(player.setVolume).toHaveBeenCalled();
});
it("dispatches the afterPlayerReady event", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
//when
triggerOnReady();
//then
expect(afterPlayerReadySpy).toHaveBeenCalled();
});
it("enables setShuffle", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
//when
triggerOnReady();
//then
expect(player.setShuffle).toHaveBeenCalledWith(true);
});
it("resets play order", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
//when
triggerOnReady();
//then
expect(player.playVideoAt).toHaveBeenCalledWith(0);
});
it("stops video after resetting play order", function() {
//given
afterPlayerReadySpy = initVideoPlayer();
//when
triggerOnReady();
//then
expect(player.stopVideo).toHaveBeenCalled();
});
});
});
Let’s do these one at a time now. They are all for testing addPlayer, but we only have indirect access to addPlayer via the init function.
Here’s how the “needs a playlist” test currently looks.
it("needs a playlist", function() {
//given
const playlist = null;
//when
const addPlayerWithNoPlaylist = function() {
//videoPlayer.addPlayer(video, playlist);
videoPlayer.init(videoIds, playlist);
};
//then
expect(addPlayerWithNoPlaylist).toThrowError("A playlist is required.");
});
The videoPlayer.init() function call doesn’t have a second argument, so playlist must be removed from there. That also means removing the const playlist line.
For the expected error to occur, videoIds needs to be an empty array, so in addPlayerWithNoPlaylist function use const to define videoIds as being an empty array.
And lastly, the addPlayerWithNoPlaylist could do with being renamed to something more suitable, such as initWithNoVideos.
1 Like
Next with a different test called “has a playlist”, I’ll go a bit deeper into the thought process of what I do there.
Focus on one thing at a time
When there are several tests that are failing, it can really help to focus only on the one test so that I’m not distracted by all of the other things going wrong. To achieve that I rename it to be fit instead, which forces only those it tests to be run.
// it("has a playlist", function() {
fit("has a playlist", function() {
Later on when the test runs properly again, I rename it back to it so that all of the tests get run.
Here is the test as it currently is. There are several issues in there that need to be resolved.
fit("has a playlist", function() {
//given
options = undefined;
const playlist = ["0dgNc5S8cLI", "mnfmQe8Mv1g"];
//when
//videoPlayer.addPlayer(video, playlist);
videoPlayer.init(videoIds, playlist);
//then
const playerVars = options.playerVars;
expect(playerVars.playlist).toBe(playlist);
});
Examination
Examination now takes place. No touching of any of the code occurs. This is all investigation to find issues, and learn what I can about how things are currently being done, and think about ways to possibly improve them.
When looking through that I am reassured that there is a given, when, and then section.
The givens are information, the when are actions, and the then are what is expected to have changed after those actions occurred.
In the given section are options and playlist.
The options are cleared out because one of the actions is supposed to set that to a different value which gets checked in the then section. All of that is good.
The playlist is a problem though, because an array of strings are video ids, not a playlist. A playlist is quite different from that, being a single string containing comma-separated video ids.
Looking further about how the playlist is used, it is being passed as the second argument to the init method. It looks like that is a mistake from you renaming a previous addPlayer method to init instead. Those playlist parts will all need to be removed.
The commented addPlayer line also needs to be removed.
Because we a have defined suitable videoIds and playlist variables further up in the test, we don’t want to define those anywhere in the test.
I haven’t touched any of the test yet. I have only scanned through it while recording a stream of consciousness here about my observations.
Improving the test
The first thing I would do is to remove the commented out addPlayer line, remove the playlist variable that’s defined in the given section, is followed by removing the playlist second argument to the init method.
Check your progress
This is when I run the test again to see the impact of what I’ve done.
The test does everything that it’s expected to do, but there’s one final issue that’s happening from something outside of the test.
Expected '0dgNc5S8cLI,CHahce95B1g,mnfmQe8Mv1g,2VwsvrPFr9w' to be '0dgNc5S8cLI,mnfmQe8Mv1g,CHahce95B1g,2VwsvrPFr9w'.
Examination
Everything in that test is working well, but we have a conflict between the videoIds and the playlist. Let’s take a look at where those are defined.
afterAll(function() {
videoPlayer.init = videoPlayer.originalInit;
});
let options;
let player;
let playlist = "0dgNc5S8cLI,mnfmQe8Mv1g,CHahce95B1g,2VwsvrPFr9w";
const videoIds = ["0dgNc5S8cLI", "CHahce95B1g", "mnfmQe8Mv1g", "2VwsvrPFr9w"];
First in terms of ordering, I would have videoIds first, the playlist, then player, then options because that is the order in which they are used by the videoPlayer code.
Fixing the root cause
Order those defined variables vertically as discussed, and order the playlist id’s horizontally to match the videoIds order, and everything there looks and works great.
Finishing up
After that the test passes, and it is the only test that runs so I go back down to the test to rename fit back to it, run the test again, and give the test one last look over to check if anything else comes obvious to mind to fix up in there. Everything looks good in there after the changes, so it’s on to the next issue in a test to resolve.
That just leaves one last failing test to go, which I’ve left for last as it’s the trickiest of the lot to resolve.
https://jsfiddle.net/xe82darc/1/
it("needs a playlist", function() {
//given
const videoIds= []
//when
const initWithNoVideos = function() {
videoPlayer.init(videoIds);
};
//then
expect(addPlayerWithNoPlaylist).toThrowError("A playlist is required.");
});
Still getting 3 failures.
https://jsfiddle.net/xe82darc/4/
const videoIds = [
"0dgNc5S8cLI",
"mnfmQe8Mv1g",
"CHahce95B1g",
"2VwsvrPFr9w"
];
let playlist = "0dgNc5S8cLI,mnfmQe8Mv1g,CHahce95B1g,2VwsvrPFr9w";
let player;
let options;
Here is one of the tests, the “needs a playlist” test.
it("needs a playlist", function() {
//given
const videoIds = []
//when
const initWithNoVideos = function() {
videoPlayer.init(videoIds);
};
//then
expect(addPlayerWithNoPlaylist).toThrowError("A playlist is required.");
});
The error says: ReferenceError: addPlayerWithNoPlaylist is not defined
You have a initWithNoVideos() function that isn’t being used, and addPlayerWithNoPlaylist that cannot be found anywhere.
What you need to do there is to rename addPlayerWithNoPlaylist to be initWithNoVideos and things will work.
1 Like
Here is the other test, the “has a playlist” test.
it("has a playlist", function() {
//given
options = undefined;
const playlist = ["0dgNc5S8cLI", "mnfmQe8Mv1g"];
//when
videoPlayer.init(videoIds);
//then
const playerVars = options.playerVars;
expect(playerVars.playlist).toBe(playlist);
});
The const playlist line in there is doing no good, because earlier in the code are the videoIds and playlist variables that are being referred to instead. So delete that const playlist line.
That then leaves you with the following error:
Expected '0dgNc5S8cLI,mnfmQe8Mv1g,CHahce95B1g,2VwsvrPFr9w' to be '0dgNc5S8cLI,mnfmQe8Mv1g, CHahce95B1g,2VwsvrPFr9w'.
Let’s line them both up so that it’s easier to see where and how they differ.
Expected: '0dgNc5S8cLI,mnfmQe8Mv1g,CHahce95B1g,2VwsvrPFr9w'
Actual: '0dgNc5S8cLI,mnfmQe8Mv1g, CHahce95B1g,2VwsvrPFr9w'
There’s something wrong with the actual part, from the toBe(playlist) part of the test.
Let’s take a look at where videoIds and playlist are defined, further up in the code:
const videoIds = [
"0dgNc5S8cLI",
"mnfmQe8Mv1g",
"CHahce95B1g",
"2VwsvrPFr9w"
];
let playlist = "0dgNc5S8cLI,mnfmQe8Mv1g, CHahce95B1g,2VwsvrPFr9w";
Well there’s the problem. Remove the space from after that comma and things will be going well there too.
1 Like
1 failure is left: https://jsfiddle.net/fcz2v4s1/
Expected function to throw TypeError with a message matching /Element needs a video classname/, but it threw TypeError with message ‘videoIds.join is not a function’.
That’s the tough one, because it means removing the video element from the HTML code, and I don’t know yet if it can be put back again. Does other code in the test run when an exception occurs? I don’t know yet, so some learning needs to occur here.
First of all, when an exception occurs, does any code get run that occurs after that exception?
fit("can show a log message after an exception.", function () {
function errorFunc() {
throw new Error();
}
console.log("before error thrown");
expect(errorFunc).toThrow();
console.log("after error thrown");
});
I’m seeing both console statements in the browser console, which is good news. After an exception has occurred, the rest of that test continues to run so it will be possible to remove the video element, have the exception occur, then put the video element back in place. That is good news for the future.
Here is the test that we are updating, due to the addPlayer method no longer existing.
it("addPlayer requires a video element", function() {
//given
const badVideo = document.createElement("div");
//then
function badArgument() {
//videoPlayer.addPlayer(badVideo, playlist);
videoPlayer.init(badVideo, playlist);
}
expect(badArgument).toThrowError(TypeError, /Element needs a video classname/);
});
There have been more problems added to that test from previous fiddling, so it may be best to start over from scratch.
it("addPlayer requires a video element", function() {
function initVideoPlayer() {
videoPlayer.init(videoIds);
}
expect(initVideoPlayer).toThrowError("Element needs a video classname.");
});
https://jsfiddle.net/4wcdxo7n/1/
Expected function to throw an Error.
it("addPlayer requires a video element", function() {
function initVideoPlayer() {
videoPlayer.init(videoIds);
}
expect(initVideoPlayer).toThrowError("Element needs a video classname.");
});
I do want you to have more involvement here than just blindly copy/pasting the code.
That’s why try to run this similar to paired programming, where the navigator thinks and explains, and the driver does the coding.
The test now expect for an error to be thrown, but that isn’t happening yet.
Please add given/when/then sections to the test, and put the initVideoPlayer() function in the when section, and the expect statement in the then section.
We will then add something to the given section.
That given, is where we are going to remove the video element from the HTML code.
1 Like
https://jsfiddle.net/fp5xw3md/1/
it("addPlayer requires a video element", function() {
//given
//when
function initVideoPlayer() {
videoPlayer.init(videoIds);
}
//then
expect(initVideoPlayer).toThrowError("Element needs a video classname.");
});
In the HTML element, here is the where the video is found.
<div class="wrap hide">
<div class="video video-frame"></div>
</div>
We need to remove that video for this test to pass.
In the given section of the test, use JavaScript to remove that video element.
When that tests passes, we can then put back the video element (which I’ll take you through) and all of the tests will be working and passing.
But first, we need to remove that video element.