Adding tests to video player code

The spinner provides a valuable service, and that is to explain to people using the page that they aren’t allowed to click the cover until the video player has loaded.

Loading the video always costs time. That time can either occur before the click, or after the click, but it must occur. Doing it before the click results in less annoyance for the person using the page then when it happens afterwards.

2 Likes

With the tests, the utils code is easy to put some tests together for.

utils.test.js

/*global utils beforeEach describe it expect */
describe("Utils", function () {
    let div;
    beforeEach(function () {
        div = document.createElement("div");
    });
    describe("show", function () {
        beforeEach(function () {
            div.classList.add("hide");
        });
        it("can show a hidden element", function () {
            utils.show(div);
            expect(div.classList.contains("hide")).to.equal(false);
        });
    });
    describe("hide", function () {
        beforeEach(function () {
            div.classList.remove("hide");
        });
        it("can hide an element", function () {
            utils.hide(div);
            expect(div.classList.contains("hide")).to.equal(true);
        });
    });
});

Usually tests are performed using the command line and an environment like Node. Because most of these tests are going to be browser based, I’m going to do some standalone testing on a separate page, where I have a separate test page for each module, one called utils-test.html, one called spinner-test.html, one called manage-cover-test.html and so on, as well as a separate all-tests.html page that combines them all together.

The spinner-test.html page is also easy to setup using the already existing spinner.test.js code from earlier on. Next up is the manageCover tests.

1 Like

The manageCover code now only has a single method that’s used, called init.

When testing that, we can check that init works in two different situations, one where cover is provided by itself with no callback, and also where cover is provided with a callback.

With the callback, that is where the chai-spies library becomes useful. We can use it to easily check that the callback was called.

manage-cover.js

/*global manageCover chai beforeEach describe it expect */
describe("Manage cover", function () {
    let documentFragment;
    let cover;
    let wrapper;
    beforeEach(function () {
        cover = document.createElement("div");
        wrapper = document.createElement("div");
        documentFragment = document.createDocumentFragment();
        documentFragment.appendChild(cover);
        documentFragment.appendChild(wrapper);
    });
    it("inits a cover", function () {
        manageCover.init(cover);
        expect(cover.classList.contains("hide")).to.equal(false);
        cover.click();
        expect(cover.classList.contains("hide")).to.equal(true);
    });
    it("inits a cover with a click callback", function () {
        const clickCallback = function () {
            return;
        };
        const clickCallbackSpy = chai.spy(clickCallback);
        manageCover.init(cover, clickCallbackSpy);
        cover.click();
        expect(clickCallbackSpy).to.have.been.called();
    });

There is trouble though when no callback is used. Looking at the manageCover code, that could do with a minor overhaul to properly support events.

When we init manageCover we can pass the event handlers to an addEvents function. Using an eventHandlers object can be more useful for that, so we should update the test to define what we expect to occur.

manage-cover.test.js

        manageCover.init(cover, {
            afterClickCover: clickCallbackSpy
        });

We can now update the manageCover code so that it accepts eventHandlers, and does something useful with them.

manage-cover.js

    function init(cover, eventHandlers = {}) {
        initEvents(eventHandlers);
        ...
    }

In that initEvents function we can create our own events and store aside their handlers:

manage-cover.js

const manageCover = (function makeManageCover() {
    const events = {};
    const config = {};
...
    function initEvents(eventHandlers) {
        events.afterClickCover = new Event("afterClickCover");
        config.afterClickCover = eventHandlers.afterClickCover;
    }

And then to connect things together, we can add the event listener and dispatch it at the right time.

manage-cover.js

    function clickCoverHandler(evt) {
        ...
        cover.dispatchEvent(events.afterClickCover);
    }
    function init(cover, eventHandlers) {
        ...
        cover.addEventListener("afterClickCover", config.afterClickCover);
    }

That is the three-step process that’s usually used when defining any events.

We just need to update the manageCover code so that it uses an object for the event handler, letting us more clearly see which type of event it will be used for.

manage-cover.js

    function afterClickCallback(evt) {
        ...
    }
...
        afterPlayerReady: function (evt) {
            ...
            manageCover.init(cover, {
                afterClickCover: clickCallback
            });
        }

That causes the manage-cover test page to pass the test, telling us that the manageCover code is ready to be used.

We just need to update addPlayers to use the afterClickCover property:

add-players.js

        afterPlayerReady: function (evt) {
            ...
            manageCover.init(cover, {
                afterClickCover: afterClickCallback
            });
        }

And all is good. There are still other things that manageCover does that should be tested, but the code at https://jsitor.com/1xqkXKIrsv has been updated and I’ll come back to further tests on manageCover shortly.

The other thing that manageCover does is to hide the cover when it’s clicked.

Currently we are testing that at the same time as init. We really should separate those. First we check that init can occur without throwing any errors:

    it("inits a cover", function () {
        expect(function testInit() {
            manageCover.init(cover);
        }).to.not.throw();
    });

And then later on we can check that the cover gets hidden when clicked.

    it("hides the cover when clicked", function () {
        manageCover.init(cover);
        cover.click();
        expect(cover.classList.contains("hide")).to.equal(true);
    });

Test one thing at a time, as that makes it easier later on to get information when things break.

The manageCover code now has all of its tests updated at https://jsitor.com/1xqkXKIrsv, and it looks like videoPlayer code is the next lot to test.

1 Like

I’ve been putting off testing videoPlayer because it’s a large amount of code, which also means a large number of tests. I’ve been reorganizing things though so that it’s easier to test, and with the local separation of the code that I’ve been doing, I can make good use of browser tools too such as Coverage to ensure that I’m testing as much as I reasonably can about the code.

There are many impediments to testing the videoPlayer code, but we can chip away at them one at a time.

First of all the testing code can be run both after the player_api code has already been loaded, and when it hasn’t been loaded at all. We can use a playerAPILoaded boolean to easily figure that out.

video-player.test.js

describe("videoPlayer", function () {
    let hasPlayerAPI = false;
    beforeEach(function () {
        const scripts = Array.from(document.querySelectorAll("script"));
        scripts.find(function (script) {
            hasPlayerAPI = script.src.includes("player_api");
        });
    });

That means we can now do a separate test when the player API hasn’t yet been loaded:

    describe("init", function () {
        if (hasPlayerAPI === false) {
            it("can init", function () {
                videoPlayer.init();
            });
        }

But before expecting anything useful from it, we need to fix an issue when there’s no callback.
video-player.js:23 Uncaught TypeError: config.onIframeReady is not a function

Let’s tidy up the videoPlayer events using what we learned from the manageCover code.

    function init(callback, initConfig) {
        initEvents(callback, initConfig);
        loadIframeScript();
        window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
    }

The initEvents function is where we setup the events and the handlers for them.

    function initEvents(callback, initConfig = {}) {
        events.afterPlayerReady = new Event("afterPlayerReady");
        events.afterPlayerStateChange = new Event("afterPlayerStateChange");
        config.onIframeReady = callback;
        config.afterPlayerReady = initConfig.afterPlayerReady;
        config.afterPlayerStateChange = initConfig.afterPlayerStateChange;
    }

We can then add onIframeReady to an available element. As no other elements can be guaranteed to exist, we can use document here.

    function init(callback, initConfig) {
        ...
        document.addEventListener("onIframeReady", events.onIframeReady);
    }

And in the onYouTubeIframeAPIReady function, we can dispatch that event.

    function onYouTubeIframeAPIReady() {
        document.dispatchEvent(events.onIframeReady);
    }

That way it keeps working even if no handler has been given.

The code at https://jsitor.com/1xqkXKIrsv has been updated, and the testing continues.

1 Like

None of the videos open when clicked on.

Another test is required now to ensure that the callback code gets run.

We can use a spy to help us test that.

    describe("init", function () {
        if (hasPlayerAPI === false) {
            it("can init", function () {
                const callback = function () {
                    return;
                };
                let callbackSpy = chai.spy(callback);
                videoPlayer.init(callbackSpy);
                expect(callbackSpy).to.have.been.called();
            });
        }

Currently that test fails, so let’s update the code so that it passes.

Looking at the initEvents function I notice that event for onIframeReady hasn’t been defined. I should rename that to be afterIframeReady to be consistent with our naming scheme.

    function initEvents(callback, initConfig) {
        events.afterIframeReady = new Event("afterIframeReady");
        ...
        config.afterIframeReady = callback;
        ...
    }

The onYouTubeIframeAPIReady event that gets called when the iframe API loads, is where the after event is dispatched:

    function onYouTubeIframeAPIReady() {
        document.dispatchEvent(events.afterIframeReady);
    }

And, the init function is where we tell the event listener which code to run:

    function init(callback, initConfig = {}) {
        initEvents(callback, initConfig);
        ...
        document.addEventListener("afterIframeReady", config.afterIframeReady);
    }

The test still says that the spy hasn’t been called, and that’s because the test itself runs before the onYouTubeIframeAPIReady event is called. We can move the expect statement into the callback itself to deal with that.

            it("can init", function () {
                const callback = function () {
                    expect(callbackSpy).to.have.been.called();
                };
                callbackSpy = chai.spy(callback);
                videoPlayer.init(callbackSpy);
            });

The tests now work, the separate page that I have now works, and the updated code at https://jsitor.com/1xqkXKIrsv also works too.

1 Like

But, it’s not just about having code work. Tests are there to ensure that you have feedback about what the code does, so that you can get early and specific feedback when things go wrong. In light of that, the work on more videoPlayer tests continues.

There isn’t much else to test in regard to init, so now we move on to other things that happen after init.

The stand-alone tests don’t have player_api already loaded, so we need to check for that and carry on the tests when it exists. We can’t use the onYouTubeIframeAPIReady event. That only works with the standalone test, but does nothing when the player_api is already loaded.

Instead, we can use setInterval to check for YT.loaded, which is set to 1 when it’s loaded. We can wait until then, and then trigger the rest of the tests that use the API.

    describe("after init", function () {
        before(function (done) {
            const timer = setInterval(function () {
                if (window.YT && window.YT.loaded) {
                    clearInterval(timer);
                    done();
                }
            }, 200);
        });
    });

What else about initializing videoPlayer needs checking? Well it makes onYouTubeIframeAPIReady available, so we should check that exists.

        it("makes onYouTubeIframeAPIReady available", function () {
            expect(window.onYouTubeIframeAPIReady).to.not.equal(undefined);
        });

There isn’t much else to do with initing videoPlayer that needs to be checked.

Checking the Coverage tab in my browser tools, I see that most of the videoPlayer code still remains untested. That’s a good thing a this stage because we haven’t done much testing of the videoPlayer code, and it helps to give us direction. The next videoPlayer tests that I add can help to improve on that.

The methods of videoPlayer (in the order that we can check them) are init, addPlayer, setDefaults, players, show getOptions, play.

When it comes to adding a player, I don’t want to have to rely on YT. There’s no guarantee that it will be loaded before the testing code runs, but more importantly, we are not testing the YT code. As such it’s important for us to simulate what YT does when testing, so that the tests aren’t slowed down by what the real one does.

The addPlayer method takes three parameters:

    function addPlayer(videoWrapper, settings = {}, eventHandlers = {}) {

When testing it’s helpful to test failing conditions first, to put off reaching for the prize until everything else around it has been checked. As such, we should start with testing when no videoWrapper is given.

I can expect addPlayer to throw an error when it has no videoWrapper. I don’t care about which type of error it is, I just want to confirm that it does throw an error.

        it("needs an element as the videoWrapper", function () {
            expect(function () {
                videoPlayer.addPlayer();
            }).to.throw();
        });

That’s what happens with no parameter. We should also check what happens when it’s given an element that isn’t a suitable videoWrapper. It throws an error because it can’t find a .video element.

        it("needs a .video child", function () {
            const div = document.createElement("div");
            expect(function () {
                videoPlayer.addPlayer(div);
            }).to.throw();
        });

Let’s try again and give it a .video child.

        it("add a player", function () {
            const videoWrapper = document.createElement("div");
            const video = document.createElement("div");
            video.classList.add("video");
            videoWrapper.appendChild(video);
            videoPlayer.addPlayer(videoWrapper);
        });

We are now given a YT doesn’t exist error. That’s good, for we can simulate what is needed from the YT object. By following the list of errors, we build up a Player method that looks like this:

            const iframe = document.createElement("div");
            window.YT = {
                Player: function () {
                    return {
                        h: iframe
                    };
                }
            };

The objective here is to build up only the minimal required to get things working. Attempting to replicate the behaviour of the youtube iframe API is completely unacceptable. We are not testing that API, but our own code instead.

The last part of this test is how do we check that things went well? We can know that by checking if the YT.Player method was called. We can use chai.spy to spy on that, and check that it was called with the video.

            function Player() {
                return {
                    h: iframe
                };
            }
            const PlayerSpy = chai.spy(Player);
            window.YT = {
                Player: PlayerSpy
            };
            videoPlayer.addPlayer(videoWrapper);
            expect(PlayerSpy).to.have.been.called.with(video);

That’s a lot of stuff in the test. We can move most of it out to a beforeEach function, so that the test only contains a minimum of what is required.

        let videoWrapper;
        let video;
        let PlayerSpy;
        beforeEach(function () {
            videoWrapper = document.createElement("div");
            video = document.createElement("div");
            video.classList.add("video");
            videoWrapper.appendChild(video);
            const iframe = document.createElement("div");
            function Player() {
                return {
                    h: iframe
                };
            }
            PlayerSpy = chai.spy(Player);
            window.YT = {
                Player: PlayerSpy
            };
        });
        ...
        it("add a player", function () {
            videoPlayer.addPlayer(videoWrapper);
            expect(PlayerSpy).to.have.been.called.with(video);
        });

And lastly, so that we don’t interfere with YT when the test runs on an already running page, we’ll cache aside the real YT and restore it afterwards.

    describe("addsPlayer", function () {
        let cache = {};
        ...
        before(function () {
            cache.YT = window.YT;
        });
        ...
        after(function () {
            window.YT = cache.YT;
        });
        ...
    });

Now that we’ve tested what happens with one of the parameters, next up is to test what things change with the other parameters.

1 Like

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.

1 Like

As the task of the videoPlayer code is to do everything properly according to the youtube iframe API, it doesn’t make sense for it to have to deal with separating out our custom settings object. Fortunately we can move that problem back to the addPlayer code.

We can achieve that with a gradual set of changes. First, we tell our tests about the new structure that we require from the videoPlayer code.

video-player.test.js

        it("should update playerOptions parameters", function () {
            const options = {
                playerVars: {
                    start: 4
                },
                videoid: "9phZWySNsXY"
            };
            videoPlayer.addPlayer(videoWrapper, options);
            const player = iframe.player;
            const playerOptions = videoPlayer.getOptions(player);
            expect(playerOptions.videoid).to.equal(options.videoid);
            const start = playerOptions.playerVars.start;
            expect(start).to.equal(options.playerVars.start);
        });

That causes the test to fail, so we update videoPlayer to make the test pass. That change is just to have playerOptions properly in the function signature as a parameter, and not use createPlayerOptions

video-player.js

    function addPlayer(videoWrapper, playerOptions = {}, eventHandlers = {}) {
        const video = videoWrapper.querySelector(".video");
        const player = createPlayer(video, playerOptions, eventHandlers);
        videoWrapper.player = player;
    }

That all works, except for a test about adding a bad property. Because the youtube API ignores bad properties, and removal of bad properties is now going to be done by the addPlayer code instead, we can move that code to the tests for addPlayer.

The addPlayer test requires some setup, so I’ve made that nice and clear by moving that setup into an addTestCoverAndWrapper function.

add-player.test.js

describe("addPlayer", function () {
    describe("add", function () {
        let coverSelector;
        let videoWrapper;
        let addPlayerSpy;
        function addTestCoverAndWrapper() {
            const parent = document.createElement("div");
            parent.classList.add("testparent");
            const testCover = document.createElement("div");
            testCover.classList.add("testcover");
            coverSelector = ".testcover";
            videoWrapper = document.createElement("div");
            parent.appendChild(testCover);
            parent.appendChild(videoWrapper);
            document.body.appendChild(parent);
        }
        beforeEach(function () {
            addTestCoverAndWrapper();
            const addPlayer = function () {
                return;
            };
            addPlayerSpy = chai.spy(addPlayer);
            window.videoPlayer = {
                addPlayer: addPlayerSpy
            };
        });
        afterEach(function () {
            document.querySelector(".testparent").remove();
        });
        it("shouldn't add a bad property to playerOptions", function () {
            const settings = {
                badProp: "bad property"
            };
            addPlayer.add(coverSelector, settings);
            expect(addPlayerSpy).to.have.been.called.with(
                videoWrapper,
                {
                    playerVars: {}
                }
            );
        });
    });
});

The test currently fails because addPlayer is not yet removing the bad properties. We can copy that separateOptions function out of videoPlayer, and put it in addPlayer instead.

add-player.js

    function addEvents(cover) {
        ...
    }
    function separateOptions(settings) {
        ...
    }
    function afterReadyWrapper(cover, afterPlayerReady) {
        ...
    }

We can now tell the addPlayer code to use that, and everything there should be all good.

add-player.js

    function add(coverSelector, settings = {}) {
        ...
        const playerOptions = separateOptions(settings);
        ...
        videoPlayer.addPlayer(videoWrapper, playerOptions, {
            ...
        });

The test works. We still need to test though that combined settings get properly separated out. That should be easy to test now.

add-player.test.js

        it("separates settings for playerOptions and playerVars", function () {
            const settings = {
                start: 4,
                videoid: "9phZWySNsXY"
            };
            addPlayer.add(coverSelector, settings);
            expect(addPlayerSpy).to.have.been.called.with(
                videoWrapper,
                {
                    playerVars: {
                        start: settings.start
                    },
                    videoid: settings.videoid
                }
            );
        });

That is all confirmed to be working fine.

The last thing that needs to be done is to clean up the videoPlayer code, to remove the use of separateOptions. There, we can remove the separateOptions function and rename settings so that it’s clear that we are sing the options to get a new playerOptions that combines the default options with the options we’ve provided.

video-player.js

    function createPlayerOptions(options) {
        const defaultOptions = {
            height: 207,
            width: 277
        };
        const playerOptions = Object.assign({}, defaultOptions, options);
        playerOptions.playerVars = Object.assign({}, playerOptions.playerVars);
        return playerOptions;
    }
    function addPlayer(videoWrapper, options = {}, eventHandlers = {}) {
        const video = videoWrapper.querySelector(".video");
        const playerOptions = createPlayerOptions(options);
        const player = createPlayer(video, playerOptions, eventHandlers);
        videoWrapper.player = player;
    }

The videoPlayer code now behaves more properly, and the addPlayer code helps to clean up the combination of playerOptions and playerVars.

After that temporary diversion with addPlayer, we head back to carry on with the videoPlayer tests, this time in regard to events.

1 Like

With the videoPlayer handlers, here’s the videoPlayer function properties:

video-player.js

    function addPlayer(videoWrapper, options = {}, eventHandlers = {}) {

In the addEvents function we can see that it’s both the afterPlayerReady and afterPlayerStateChange events that we can use. Let’s start with afterPlayerReady.

video-player.test.js

        it("supports the afterPlayerReady event", function () {
            const options = {};
            const afterPlayerReady = function () {
                return;
            };
            const afterPlayerReadySpy = chai.spy(afterPlayerReady);
            const eventHandlers = {
                afterPlayerReady: afterPlayerReadySpy
            };
            videoPlayer.addPlayer(videoWrapper, options, eventHandlers);
            expect(afterPlayerReadySpy).to.have.been.called();
        });

That doesn’t yet work, because our fake player needs to trigger the onReady event. When we do that we can move the afterPlayerReadySpy into the beforeEach function too.

video-player.test.js

        let afterPlayerReadySpy;
        ...
        beforeEach(function () {
            ...
            function fakePlayer(ignore, playerOptions) {
                playerOptions.events.onReady();
                return {
                    ...
                };
            }
            ...
            const afterPlayerReady = function () {
                return;
            };
            afterPlayerReadySpy = chai.spy(afterPlayerReady);
        });

The test is now simpler than before.

video-player.test.js

        it("supports the afterPlayerReady event", function () {
            const eventHandlers = {
                afterPlayerReady: afterPlayerReadySpy
            };
            videoPlayer.addPlayer(videoWrapper, {}, eventHandlers);
            expect(afterPlayerReadySpy).to.have.been.called();
        });

Problems still exist from this test, because the onReady code expects things that aren’t yet being done. We can get the playerOptions from the player to the event target and use that.

        it("supports the afterPlayerReady event", function () {
            const eventHandlers = {
                afterPlayerReady: afterPlayerReadySpy
            };
            videoPlayer.addPlayer(videoWrapper, {}, eventHandlers);
            const playerOptions = videoPlayer.getOptions(iframe.player);
            playerOptions.events.onReady({
                target: iframe.player
            });
            expect(afterPlayerReadySpy).to.have.been.called();
        });

We now find that some method expected to exist on the player are being called, so we should add stub versions of those:

video-player.test.js

                const player = {
                    h: iframe,
                    i: {
                        j: playerOptions
                    },
                    pauseVideo: function () {
                        return;
                    },
                    playVideoAt: function () {
                        return;
                    },
                    setShuffle: function () {
                        return;
                    },
                    setVolume: function () {
                        return;
                    }
                };

And everything now works. We can do something similar now to check the afterPlayerStateChange event too.

We add a spy for afterPlayerStateChange:

video-player.test.js

        let afterPlayerReadySpy;
        let afterPlayerStateChangeSpy;
        ...
        beforeEach(function () {
            ...
            const afterPlayerStateChange = function () {
                return;
            };
            afterPlayerStateChangeSpy = chai.spy(afterPlayerStateChange);
        });

And that’s all working well.

I am left puzzled though about why the videoPlayer code uses videoWrapper instead of just the video. Looking through the code I see function parameters of videoWrapper, player, iframe, and video.

Next up I’ll see if there’s a way to make that more consistent.

1 Like

The videoPlayer code uses a lot of references to videoWrapper, player, iframe, and video.

At minimum it could only be video to set things up, and player from there on out to do everything.
We definitely want to remove videoWrapper though, because the videoPlayer should not need to know anything about that.

The tests can help us to achieve that change in a reliable manner. Instead of videoWrapper, we can use video when initializing a player, and iframe afterwards.

Because this is quite a significant change to the function signature, we can do it slowly and gradually, by first introducing the new variable and when things are working from that, removing the old one.

We can get started by updating the video test and passing in video as the 4th parameter. We will eventually switch around the first and fourth parameters, but we’ll just get things working first with video as the fourth.

video-player.test.js

        it("add a player", function () {
            videoPlayer.addPlayer(videoWrapper, null, null, video);
            expect(PlayerSpy).to.have.been.called.with(video);
        });

The tests still pass. We can now add video to the function parameter, and remove the need for the separate video assignment.

video-player.js

    function addPlayer(videoWrapper, options = {}, eventHandlers = {}, video) {
        // const video = videoWrapper.querySelector(".video");

That causes other tests that used videoWrapper to break, so we fix those now.

video-player.test.js

            videoPlayer.addPlayer(videoWrapper, options, undefined, video);
...
            videoPlayer.addPlayer(videoWrapper, {}, eventHandlers, video);

The tests go back to passing. We can now replace videoWrapper with video, and get rid of that end video parameter.

video-player.js

    function addPlayer(video, options = {}, eventHandlers = {}) {

video-player.test.js

            videoPlayer.addPlayer(video);
...
            videoPlayer.addPlayer(video, options);
...
            videoPlayer.addPlayer(video, {}, eventHandlers);

And some things now break. Fortunately we have the tests to tell us when things are working again.

In this case it is where the player is being added to the videoWrapper.

video-player.js

    function addPlayer(video, options = {}, eventHandlers = {}) {
        ...
        videoWrapper.player = player;
    }

Instead of doing that, we should just return the player itself and have other code outside of the videoPlayer code update the wrapper.

Now that player is being returned, we have a more reliable way for tests to check the player, so we can ensure that iframe properly exists on there too.

video-player.test.js

        it("add a player", function () {
            const player = videoPlayer.addPlayer(video);
            expect(PlayerSpy).to.have.been.called.with(video);
            expect(player.h).to.equal(iframe);
        });

We can also remove the videoWrapper variable from the tests, as that’s not needed anymore.

    describe("addsPlayer", function () {
        let cache = {};
        // let videoWrapper;
        let video;
        ...
        beforeEach(function () {
            // videoWrapper = document.createElement("div");
            video = document.createElement("div");
            video.classList.add("video");
            // videoWrapper.appendChild(video);

That is the videoPlayer code all updated. We just need to bring the other code that interacts with it up to speed too. In the next post I’ll update addPlayer to match, and we’ll see if the updates work together.

1 Like

The addPlayer tests spy on how videoPlayer.addPlayer is being used, and gives us a good visibility into how videoWrapper is used there.

add-player.test.js

            addPlayer.add(coverSelector, settings);
            expect(addPlayerSpy).to.have.been.called.with(
                videoWrapper,
                {
                    playerVars: {}
                }
            );

We want to use video instead of videoWrapper, That means creating a video element and adding it to videoWrapper.

add-player.test.js

    describe("add", function () {
        let coverSelector;
        let videoWrapper;
        let video;
        let addPlayerSpy;
...
            videoWrapper = document.createElement("div");
            video = document.createElement("div");
            video.classList.add(".video");
            videoWrapper.appendChild(video);

We can now update the tests so that they expect video instead of videoWrapper.

            addPlayer.add(coverSelector, settings);
            expect(addPlayerSpy).to.have.been.called.with(
                video,
                {
                    playerVars: {}
                }
            );

The tests fail of course, because we need to update the addPlayer code now to make that change.

add-player.js

        const videoWrapper = cover.nextElementSibling;
        const video = videoWrapper.querySelector("video");
        ...
        videoPlayer.addPlayer(video, playerOptions, {

And the tests are now all good. We only need to deal with the player side of things now. The tests also need to check that player exists on the videoWrapper.

For that, we need our fake addPlayer to return an object.

add-player.test.js

            const fakeAddPlayer = function () {
                const player = {};
                return player;
            };

And we can tell the test to expect that videoWrapper has a player that is an obect.

add-player.test.js

            addPlayer.add(coverSelector, settings);
            expect(addPlayerSpy).to.have.been.called.with(
                video,
                {
                    playerVars: {}
                }
            );
            expect(videoWrapper.player).to.be.an("object");

That test fails as it’s supposed to, and we can now update addPlayer to make the test pass.

add-player.js

        const player = videoPlayer.addPlayer(video, playerOptions, {
            ...
        });
        videoWrapper.player = player;

The tests all pass, and the video page code works, and the code at https://jsitor.com/1xqkXKIrsv is updated too.

While doing that though I did notice that player is attached to the iframe, to the videoWrapper, and it is returned from the addPlayer method too. There really shouldn’t be that many references to it so I’ll explore that next.

1 Like

In the videoPlayer code, the createPlayer function both assigns player to the iframe, and also returns player from the function. Assigning player to the iframe is a lazy thing to do in the videoPlayer code. That could easily be done from outside of the videoPlayer code instead, after which a re-evaluation can occur.

video-player.js

        const iframe = getIframe(player);
        // iframe.player = player;
        addEvents(iframe, eventHandlers);
        return player;

Do any of the tests care if we remove that iframe.player assignment? Some of them do, but in all cases that’s easily dealt with by using the returned player instead.

video-player.test.js

            // videoPlayer.addPlayer(video, options);
            // const player = iframe.player;
            const player = videoPlayer.addPlayer(video, options);

Because other parts of the code still rely on iframe.player, we can put that back in the add-player code. To test for that we should have the test code create an iframe element, and replace the video element with the iframe element.

add-player.test.js

        let video;
        let iframe;
...
            video = document.createElement("div");
            video.classList.add("video");
            iframe = document.createElement("iframe");
...
            const fakeAddPlayer = function (video) {
                video.replaceWith(iframe);
                ...
            };

We can now easily check that iframe.player exists, and leave a note about wanting to remove it.

add-player.test.js

            // One of these player references needs to be removed, maybe both.
            expect(videoWrapper.player).to.be.an("object");
            expect(iframe.player).to.be.an("object");

The test appropriately fails, and we can now update addPlayer to put iframe.player in place.

add-player.js

        videoWrapper.player = player;
        const iframe = videoWrapper.querySelector("iframe");
        iframe.player = player;

The tests now pass, and the code at https://jsitor.com/1xqkXKIrsv can now be updated.

The videoPlayer code still has other issues. It shouldn’t need to know or care anything about videoWrapper, which I’ll take a look at in the next post.

1 Like

The two remaining pieces of code in videoPlayer that uses videoWrapper are:

video-player.js

    function show(videoWrapper) {
        utils.show(videoWrapper);
    }
...
    function play(videoWrapper) {
        const player = videoWrapper.player;
        player.playVideo();
    }

The videoPlayer tests don’t care about the show function, but we really should have it elsewhere, such as in the addPlayers code.

video-player.test.js

    function afterClickCallback(evt) {
        const cover = evt.target;
        const videoWrapper = cover.nextElementSibling;
        // videoPlayer.show(videoWrapper);
        utils.show(videoWrapper);
        videoPlayer.play(videoWrapper);
    }

We can now remove the videoPlayer show method.

video-player.js

    // function show(videoWrapper) {
    //     utils.show(videoWrapper);
    // }
...
    return {
        addPlayer,
        getOptions,
        // hide,
        init,
        ...
    };

The other part to deal with is the play function.

video-player.js

    function play(videoWrapper) {
        const player = videoWrapper.player;
        player.playVideo();
    }

That would be better when just player is passed to it instead. None of the tests currently use play. The only place that it’s used is from the addPlayers code. I don’t plan to write tests for that until after doing tests for videoPlayer and addPlayer, so we should be able to just update the addPlayers code to use videoPlayer.player instead.

    function afterClickCallback(evt) {
        const cover = evt.target;
        const videoWrapper = cover.nextElementSibling;
        utils.show(videoWrapper);
        const player = videoWrapper.player;
        videoPlayer.play(player);
    }

In fact, the videoPlayer code doesn’t even need play. We can tell that play doesn’t belong there because none of the other methods of the player are exposed there. We can instead do it right from the addPlayers code instead.

add-players.js

    function afterClickCallback(evt) {
        const cover = evt.target;
        const videoWrapper = cover.nextElementSibling;
        utils.show(videoWrapper);
        const player = videoWrapper.player;
        player.playVideo();
    }

The play function can now be removed from videoPlayer

video-player.js

    function play(videoWrapper) {
        const player = videoWrapper.player;
        player.playVideo();
    }
...
    return {
        ...
        init,
        // play,
        players,
        ...
    };

That seems to be the worst of structural issues in videoPlayer dealt with, so we can now carry on with completing the tests for the videoPlayer code.

1 Like

Earlier I was saying that the methods of videoPlayer are init, addPlayer, setDefaults, players, getOptions, play. We have tests that act like documentation for init and addPlayer, and now it’s time to move on to setDefaults.

Like usual with tests, we should document what happens with no parameters and bad parameters.
No parameter just results in no change.

    describe("setDefaults", function () {
        beforeEach(function () {
            video = document.createElement("div");
            video.classList.add("video");
            iframe = document.createElement("div");
        });
        it("with no parameter results in no change", function () {
            videoPlayer.setDefaults();
            const options = {
                playerVars: {
                    start: 4
                },
                height: 360
            };
            const player = videoPlayer.addPlayer(video, options);
            const playerOptions = videoPlayer.getOptions(player);
            const playerVars = playerOptions.playerVars;
            expect(playerOptions.height).to.equal(options.height);
            expect(playerVars.start).to.equal(options.playerVars.start);
        });
    });

What happens when it’s given a bad parameter instead? Nothing seems to happen at all.

        it("ignores a bad parameter", function () {
            videoPlayer.setDefaults("bad parameter");
        });

We can now give it some proper default values, and check that one of them remains and the other gets updated by new values.

        it("overwrites default values with option values", function () {
            const defaultOptions = {
                height: 360,
                playerVars: {
                    start: 0
                },
                width: 480
            };
            videoPlayer.setDefaults(defaultOptions);
            const player = videoPlayer.addPlayer(video, options);
            const playerOptions = videoPlayer.getOptions(player);
            const playerVars = playerOptions.playerVars;
            expect(playerOptions.height).to.equal(options.height);
            expect(playerOptions.width).to.equal(defaultOptions.width);
            expect(playerVars.start).to.equal(options.playerVars.start);
        });

That should be all that we need to test in regard to default values. The rest of the tests for videoPlayer should be about as easy too, so I’ll get those done and report back about any issues that were found with them.

1 Like

Of the tests for setDefaults, getOptions, players, and play, the setDefaults method was easily tested.

The getOptions method was also easily tested, but as the addPlayer tests use getOptions I’ve placed the getOptions tests above the addPlayer tests.

The players property that accesses the list of players doesn’t seem to have much use. It’s only used by the addPlayer code, and is something that really should be in the addPlayer code instead. Updating the addPlayer code to do that doesn’t change anything about its behaviour, so no tests are needed. The update went well, and the videoPlayer code now no longer has players to worry about.

And with the play method, we got rid of that earlier on as well so no need for tests there.

The videoPlayer code is now fully tested. As confirmation, the browser tools Coverage panel helps to confirm that we have full coverage of that code too.

The code at https://jsitor.com/1xqkXKIrsv has been updated, and as we’ve been moving several things to addPlayer, I look forward to placing that code there under as much scrutiny.

While the videoPlayer tests work separately on a remote testing page, after adding the tests to the jsitor page there are issues relating to playerState. So some assumptions need to be refined in regard to that.

The test that’s messing up when run on a live page is the one that checks if onYouTubeIframeAPIReady is made available.

    describe("after init", function () {
        before(function (done) {
            const timer = setInterval(function () {
                if (window.YT && window.YT.loaded) {
                    clearInterval(timer);
                    done();
                }
            }, 200);
        });
        it("makes onYouTubeIframeAPIReady available", function () {
            expect(window.onYouTubeIframeAPIReady).to.not.equal(undefined);
        });
    });

As the init callback function is run by that, we don’t need an explicit test in regard to onYouTubeIframeAPIReady because we have other tests that already check if the callback function gets run, so we can delete that test. https://jsitor.com/1xqkXKIrsv

But how does that affect testing of the videoPlayer on the separate remote testing page? All is good even after removing that test. The browser Coverage tools still report that we have 100% coverage of the code, so all is now good.

Now that the hard part is done, tests for the other parts of the code can be put in place. As such the following sections now have a full suite of tests for them:

  • utils
  • spinner
  • manageCover
  • videoPlayer

With the above fundamentals out of the way, I can now focus on testing the addPlayer module, and possibly even push unwanted code out to the separate addPlayers module or parts elsewhere.

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.