Setting up single-player tests before adding spinner

JavaScript
2308

What is next?

https://jsfiddle.net/p8g17oab/1/

  function isVideoId(videoIds) {
    const paramType = typeof videoIds;
    const paramIsString = paramType === "string";
    return paramIsString;
  }

Changed fit to it and all the tests pass: https://jsfiddle.net/wczbo6h4/

2309

That paramIsString variable isn’t needed. It was only used to help you place the comparison in there.

You can “inline the variable”, which is a very common technique to remove unwanted variables. What you do is you copy the code that is assigned to the variable, and replace other occurrences of that variable with the copied code instead.

2310

https://jsfiddle.net/95r4ha2z/1/

  function isVideoId(videoIds) {
    const paramType = typeof videoIds;
    return paramType === "string";
  }
2311

And now, the paramType variable really isn’t needed. You can inline the paramType variable, by copying the code that is assigned to the variable, and replace paramType in the return statement with that copied code.

1 Like
2312

https://jsfiddle.net/uoyzrm36/1/

  function isVideoId(videoIds) {
    return typeof videoIds === "string";
  }

Are we up to doing the next test?

    xit("when init is a video id and has data-id", function() {
      //given
      video.dataset.id = "0dgNc5S8cLI";

      //when
      initVideoPlayer();

      //then
      expect(options.videoId).toBe("0dgNc5S8cLI");

      //cleanup
      delete video.dataset.id;
    });
2313

That test is not ready to enable yet.

Look at the description of the test, and you will see that the test doesn’t yet do what the description describes.

Also the description isn’t specific enough, and needs to explain that the video id will be used instead of the data-id. We can add a comma to the end the description and add “will use video id”.

After that, give the initVideoPlayer() function call a different video id from the one that dataset is using.

Then in the expectation, change the video id there. It shouldn’t be the data-id one, it needs to be the different one used to init the video player.

2314

https://jsfiddle.net/4L6zb9mx/

   xit("when init is a video id and has data-id, will use video id", function() {
      //given
      video.dataset.id = "0dgNc5S8cLI";

      //when
      initVideoPlayer("0dgNc5S8cLI");

      //then
      expect(options.videoId).toBe("mnfmQe8Mv1g");

      //cleanup
      delete video.dataset.id;
    });

Next return xit back to it? https://jsfiddle.net/4L6zb9mx/1/

Expected ‘0dgNc5S8cLI’ to be ‘mnfmQe8Mv1g’.

2315

No, you’re not ready yet.

The expectation needs to be the same as the video it given to initVideoPlayer, so rename the initVideoPlayer videoid to be “mnfmQe8Mv1g” instead.

2316

Now the test passes: https://jsfiddle.net/ga1v0mjr/2/

   it("when init is a video id and has data-id, will use video id", function() {
      //given
      video.dataset.id = "0dgNc5S8cLI";

      //when
      initVideoPlayer("mnfmQe8Mv1g");

      //then
      expect(options.videoId).toBe("mnfmQe8Mv1g");

      //cleanup
      delete video.dataset.id;
    });
2317

It does? Well that’s a pleasant confirmation that the videoPlayer code is doing the right thing.

What’s next from the todo list?

2318

I need to first add what we did to the video code before we can move on.

It’s not working in the video player code.

How come?

https://jsfiddle.net/rgwcfp6m/2/

manageCover.init(videoPlayer.init("2VwsvrPFr9w"));

Uncaught TypeError: Cannot read properties of undefined (reading ‘dataset’)"

That is the same error I got before making any changes to the code.

Before making changes to video player code: https://jsfiddle.net/xqu2Ly6o/1/

2319

It looks like a video argument was removed from this function:

  function getVideoIds(video, ids) {
    if (isPlaylist(ids)) {
      return ids;
    } else {
      return getVideoId();
    }
  }

Before fixing that in the live code, we need a test that duplicates the problem.

That achieves three main purposes.

  1. One is to help us to understand the problem more fully. By putting together a test, that helps us to understand more about the problem.
  2. Making the test pass provide guaranteed confirmation that the problem is solved.
  3. The test has an ongoing benefit of protecting us against further reoccurrences of the problem.
1 Like
2320

Updated video player code: https://jsfiddle.net/mhde5jco/

Still doesn’t work.

  function getVideoIds(video, ids) {
    if (isPlaylist(ids)) {
      return ids;
    } else {
      const videoId = getVideoId(video, ids);
      checkVideoIds(videoId, ids);
      return videoId;
    }
  }

What is this new test being called, where am I placing it, and am I copying a test to place in this new test?

https://jsfiddle.net/ga1v0mjr/2/

2321

That’s why I said don’t do that.

We need instead to put together a test that accurately simulates the problem, so that when we page that test pass, we can then be sure that we’ve fixed the problem.

2322

What do I need to do in the test?

What is this new test being called,
where am I placing it,
and am I copying a test to place in this new test?

https://jsfiddle.net/ga1v0mjr/2/

2323

With all of the tests passing, there is something that isn’t being appropriately tested, as is clearly the case from the live code.

In the test code at https://jsfiddle.net/ga1v0mjr/2/ we have the following getVideoIds() function:

  function getVideoIds(video, ids) {
    if (isPlaylist(ids)) {
      return ids;
    } else {
      const videoId = getVideoId(video, ids);
      checkVideoIds(videoId, ids);
      return videoId;
    }
  }

That function looks to be working properly. The function that’s in the live code is quite different.

The finger of blame now points to you for failing to properly copy over all of the videoPlayer code.

1 Like
2325

video player code works now: https://jsfiddle.net/czv3qwr7/2/

manageCover.init(videoPlayer.init("CHahce95B1g"));

function createPlaylist(videoIds) {
    return videoIds.join();
  }

  function createOptions(videoIds) {
    const options = {
      height: 360,
      host: "https://www.youtube-nocookie.com",
      width: 640
    };
    options.playerVars = {
      autoplay: 0,
      cc_load_policy: 0,
      controls: 1,
      disablekb: 1,
      fs: 0,
      iv_load_policy: 3,
      rel: 0
    };
    options.events = {
      "onReady": onPlayerReady
    };

    if (isPlaylist(videoIds)) {
      options.playerVars.loop = 1;
      options.playerVars.playlist = createPlaylist(videoIds);
    } else {
      options.videoId = videoIds;
    }
    return options;
  }

  function isVideoId(videoIds) {
    return typeof videoIds === "string";
  }

  function getVideoId(video, videoIds) {
    const videoId = video.dataset.id;
    if (isVideoId(videoIds)) {
      return videoIds;
    }
    return videoId;
  }

  function getVideoIds(video, ids) {
    if (isPlaylist(ids)) {
      return ids;
    } else {
      const videoId = getVideoId(video, ids);
      return videoId;
    }
  }

  function createVideoOptions(video, ids) {
    const videoIds = getVideoIds(video, ids);
    const options = createOptions(videoIds);
    return options;
  }

  function addPlayer(video, ids) {
    const options = createVideoOptions(video, ids);
    const player = new YT.Player(video, options);
    const iframe = getIframe(player);
    const eventHandler = config.eventHandlers.afterPlayerReady;
    iframe.addEventListener("afterPlayerReady", eventHandler);
    return player;
  }
2326

What’s next from the todo list?

Clicking on it too soon, should cause the curtain/cover not to open to the video unless it is ready.

As how it works here: https://jsfiddle.net/95oLbw3h/1/

function initCover() {
  manageCover.init(function playVideo() {
    videoPlayer.play();
  });
}
videoPlayer.afterPlayerReady = initCover;

const videoIds = [
  "0dgNc5S8cLI",
  "mnfmQe8Mv1g",
  "CHahce95B1g",
  "2VwsvrPFr9w"
];
videoPlayer.init(videoIds);

It does not work like that here: https://jsfiddle.net/czv3qwr7/3/

const videoIds = [
  "0dgNc5S8cLI",
  "mnfmQe8Mv1g",
  "CHahce95B1g",
  "2VwsvrPFr9w"
];

manageCover.init(videoPlayer.init(videoIds));

How do we reconcile how both are set up so that there is one way that it is set up?

Both codes are updated to the latest version.

Those are the only differences in both codes.

2327

What is wanted here is an onready event on the videoPlayer code, so that the manageCover code can put stuff into that onready event. That way when the videoPlayer is ready, it will run the onready event causing the manageCover to then do its thing.

First we add tests to provide support for managePlayer onready events. That support being to provide an interface by which onready events can be added.

Then we add tests for manageCover to check if onready events can be added. If the can’t then manageCover just runs as normal. If they can then manageCover delays running by adding its run code as an onready event instead.

That is the pathway. Now we step along it with slow but careful steps. The first step is adding a new describe section to the videoPlayer code called “onready” and in that section, we add a test called “add onready”

1 Like
2328

Where am I placing this new describe section?

describe("onready", function() {
    it("add onready", function() {
      //given


      //when


      //then

    });
});

Is this good where I placed it? https://jsfiddle.net/10gvc9wj/

  describe("onready", function() {
    it("add onready", function() {
      //given


      //when


      //then

    });
  });
});
setTimeout(function() {
  const videoIds = [
    "0dgNc5S8cLI",
    "mnfmQe8Mv1g",
    "CHahce95B1g",
    "2VwsvrPFr9w"
  ];

  manageCover.init(videoPlayer.init(videoIds));
}, 200);