The addPlayer method also has a settings parameter:
function addPlayer(videoWrapper, settings = {}, eventHandlers = {}) {
As usual when testing we should find out what happens when it has bad values. In this case, bad values are ones not used by the player options, and good values are used by player options, or by playerVars.
It expects an object so what happens when it’s given a string?
it("ignores a string when it should be an object", function () {
const settings = "bad string parameter";
videoPlayer.addPlayer(videoWrapper, settings);
});
Nothing seems to happen at all. It just ignores it. That’s no good, but I don’t want to have to check that every object is an object in the code. Instead, we’ve made it clear from the settings = {}
parameter that we expect it to be an object.
What happens when we have a bad property on the object?
it("...", function () {
const settings = {
badProp: "bad property"
};
videoPlayer.addPlayer(videoWrapper, settings);
});
We can make the iframe available so that we can access the player.
describe("addsPlayer", function () {
...
let iframe;
...
beforeEach(function () {
...
iframe = document.createElement("div");
...
});
...
it("...", function () {
const settings = {
badProp: "bad property"
};
videoPlayer.addPlayer(videoWrapper, settings);
const player = iframe.player;
const playerOptions = videoPlayer.getOptions(player);
console.log(playerOptions);
});
But, we cannot yet get the playerOptions from it. We need to improve our fake player so that it also puts the options where they are expected.
function fakePlayer(ignore, playerOptions) {
return {
h: iframe,
i: {
j: playerOptions
}
};
}
PlayerSpy = chai.spy(fakePlayer);
On doing that, we find that the bad property ends up in playerVars.
it("shouldn't add a bad property to playerOptions" function () {
const settings = {
badProp: "bad property"
};
videoPlayer.addPlayer(videoWrapper, settings);
const player = iframe.player;
const playerOptions = videoPlayer.getOptions(player);
console.log(playerOptions.playerVars);
expect(playerOptions.playerVars.badProp).to.equal(undefined);
});
Effectively playerVars becomes a dumping ground, and that is something to be avoided. Now that we have a failing test, we can go and update videoPlayer so that it properly takes care of this one situation, while also keeping all of the other tests passing as well.
In the separateOptions code we are properly separating out the option parameters, but everything else is just being dumped in as playerVars. Youtube gives us a full list of playerVars
supported parameters, so we should use that list to filter for things as well.
Here’s an updated separateOptions function that more properly does its job.
function separateOptions(settings) {
const options = {
playerVars: {}
};
const optionParams = ["width", "height", "videoid", "host"];
const playerVarsParams = [
"autoplay",
"cc_lang_pref",
"cc_load_policy",
"color",
"controls",
"disablekb",
"enablejsapi",
"end",
"fs",
"hl",
"iv_load_policy",
"list",
"listType",
"loop",
"modestbranding",
"origin",
"playlist",
"playsinline",
"rel",
"start",
"widget_referer"
];
Object.entries(optionParams).forEach(function separate(paramName) {
if (settings.hasOwnProperty(paramName)) {
options[paramName] = settings[paramName];
}
});
Object.entries(playerVarsParams).forEach(function separate(paramName) {
if (settings.hasOwnProperty(paramName)) {
options.playerVars[paramName] = settings[paramName];
}
});
return options;
}
We can now add a few good properties to check that they get through:
it("should update playerOptions parameters", function () {
const settings = {
start: 4,
videoid: "9phZWySNsXY"
};
videoPlayer.addPlayer(videoWrapper, settings);
const player = iframe.player;
const playerOptions = videoPlayer.getOptions(player);
expect(playerOptions.videoid).to.equal(settings.videoid);
expect(playerOptions.playerVars.start).to.equal(settings.start);
});
Hmm, that doesn’t pass. It should pass. Let’s check what happens in the separateOptions code.
Oh, a simple mistake was made. We don’t need t use Object.entries anymore, and can just loop over the array instead.
optionParams.forEach(function match(param) {
if (settings.hasOwnProperty(param)) {
options[param] = settings[param];
}
});
playerVarsParams.forEach(function match(param) {
if (settings.hasOwnProperty(param)) {
options.playerVars[param] = settings[param];
}
});
The test now works, so while it is working I can refactor that separateOptions function to make further improvements to it as I wish.
I can remove the options object that’s defined at the start of the code.
const options = {
playerVars: {}
};
Instead, I’ll just create it when looping through the arrays, using filter and reduce.
const options = optionParams.filter(function match(param) {
return settings.hasOwnProperty(param);
}).reduce(function fillParam(options, param) {
options[param] = settings[param];
return options;
}, {});
options.playerVars = playerVarsParams.filter(function match(param) {
return settings.hasOwnProperty(param);
}).reduce(function fillParam(playerVars, param) {
playerVars[param] = settings[param];
return playerVars;
}, {});
Can I make that less complex by extracting out the functions? Sure can.
function matchParam(param) {
return settings.hasOwnProperty(param);
}
function fillParam(obj, param) {
obj[param] = settings[param];
return obj;
}
function filterParams(params) {
return params.filter(matchParam).reduce(fillParam, {});
}
const options = filterParams(optionParams);
options.playerVars = filterParams(playerVarsParams);
That seems to be the addPlayer settings all tested and improved. I do have a question in regard to it though. Should the addPlayer code have to deal with our custom mashing together of options and playerVars? I’ll try to answer that one next.