Setting up single-player tests before adding spinner

asasass · June 22, 2022, 10:49pm

13 failures are still appearing.

Is that next to figure out?

This one was just fixed:

Expected spy afterPlayerReady-handler to have been called.

      // cleanup
      iframe = stubYT();
      videoPlayer.init(["0dgNc5S8cLI"]);  
    });

Here is our last video code when we are up to it: https://jsfiddle.net/oLwbtp9m/

Paul_Wilkins · June 23, 2022, 12:49am

An error still infrequently occurs when the “shouldn’t” test is the first test that runs.

We can easily experience that by forcing the “shouldn’t” test to run, using fit instead of just it

videoPlayer tests > init > shouldn’t let you play until the player is ready
TypeError: window.onYouTubeIframeAPIReady is not a function

That one happens because in the resetPlayer() function, we have not yet initialized videoPlayer.

Before calling window.onYouTubeIframeAPIReady() call videoPlayer.init() to initialize the video player.

This has been a lot of additions, but when all of the tests reliably pass we then move on to refactoring, which is where we can reorder and restructure the code to help simplify things.

asasass · June 23, 2022, 12:53am

You are having me do this? https://jsfiddle.net/okL7gncu/1/

Before calling window.onYouTubeIframeAPIReady() call videoPlayer.init() to initialize the video player.

I added: videoPlayer.init();

Now I’m receiving more failures.

 function resetPlayer() {
    window.YT = {
      Player: function createPlayer(video, playerOptions) {
        return {
          e: iframe
        };

      }
    }
    videoPlayer.init();
    window.onYouTubeIframeAPIReady();
  }

Paul_Wilkins · June 23, 2022, 3:15am

Currently that init needs function arguments. We’ve gone over this before very recently.

asasass · June 23, 2022, 3:19am

You told me to add: "0dgNc5S8cLI"

This is not working: https://jsfiddle.net/bevzau4k/2/

    videoPlayer.init("0dgNc5S8cLI");
    window.onYouTubeIframeAPIReady();
  }

videoIds doesn’t work either.

Paul_Wilkins · June 23, 2022, 6:25am

Please look at what has been done with the init statement in the “shouldn’t” test. You should see what to do from there.

It’s taken me a while to realize, but we should not have delayed improvements to the init function. Well it’s too late now, we are in the midst of things. Afterwards we can work on improving the init function so that it behaves appropriately when given no arguments.

asasass · June 23, 2022, 2:50pm

Tests passes now: https://jsfiddle.net/rjna71sy/1/

Failures are no longer showing up in the tests.

Are we now up to refactoring?

videoPlayer.init(["0dgNc5S8cLI"]);
    window.onYouTubeIframeAPIReady();
  }

Will this be part of refactoring?

Afterwards we can work on improving the init function so that it behaves appropriately when given no arguments.

As part of refactoring, I did this:

This gets removed: iframe = stubYT();

From here:

  beforeAll(function() {
    iframe = stubYT();

Then added to here:

  function initVideoPlayer(videoIds = ["0dgNc5S8cLI"]) {
    iframe = stubYT();

Then remove iframe = stubYT(); from here:

      // cleanup
      iframe = stubYT();
      videoPlayer.init();
    });

Where I received 1 failure: https://jsfiddle.net/rjna71sy/2/

Is this what we can work on fixing next?

TypeError: Cannot read properties of undefined (reading ‘nodeName’)

Paul_Wilkins · June 23, 2022, 10:30pm

Test passes Fail Pass ☐ Refactor
All tests are now passing, so it’s on to the next stage, refactoring.

Refactor the code Fail Pass ☒ Refactor

We don’t just make changes to the code without understanding what they are for.
The refactorings need to be driven by an understanding of what need to be improved.

A good way to arrive at those improvements is to understand what the problems were.

In this recent set of work since adding the “shouldn’t” test, can you create a list of all the different problems that there were?

asasass · June 23, 2022, 10:46pm

The video should be hidden.
Clicking on the play button too soon creates an error.
fixing the videoPlayer.init();
Adding videoids to the bottom.

Paul_Wilkins · June 24, 2022, 12:14am

Those are the high-level problems that we are wanting to solve, but that’s not what I’m talking about.

The situation that I’m talking about is after we creates that “shouldn’t” test to simulate the problem, we needed to do some other things to get the other tests working again. Those things are:

we needed to add a cleanup section, where stubYT() is called before initializing the video player
in resetPlayer() after assigning window.YT (similar to stubYT), we needed to initialize the video player before calling the ready event

Also, in the initVIdeoPlayer function when we initialize the video player, we call the ready event.

Those all involve stubYT, video player initialization, and the ready event in a specific sequence:

stubYT()
videoPlayer initialization
call the ready event

The cleanup of the “shouldn’t” test is only needed, I think, because when the other tests call initVideoPlayer, things aren’t being fully or properly initialized.

We should be able to completely remove that cleanup section by properly intializing things in the initVideoPlayer function.

That is what there is to be learned from this recent work, and is what helps to give focus to what this set of refactoring needs to achieve.

asasass · June 24, 2022, 12:16am

Can progress be made from here? https://jsfiddle.net/rjna71sy/2/

Where I received 1 failure

TypeError: Cannot read properties of undefined (reading ‘nodeName’)

This gets removed: iframe = stubYT();

From here:

  beforeAll(function() {
    //iframe = stubYT();

Then added to here:

  function initVideoPlayer(videoIds = ["0dgNc5S8cLI"]) {
    iframe = stubYT();

Then remove iframe = stubYT(); from here:

      // cleanup
      //iframe = stubYT();
      videoPlayer.init();
    });

Paul_Wilkins · June 24, 2022, 12:51am

What do you think is causing the failure. What needs to be done to stop the failure?

asasass · June 24, 2022, 1:00am

Inside here needs to be fixed?

  function initVideoPlayer(videoIds = ["0dgNc5S8cLI"]) {
    iframe = stubYT();
    videoPlayer.afterPlayerReady = jasmine.createSpy("afterPlayerReady-handler");
    const playFunc = videoPlayer.init(videoIds);
    window.onYouTubeIframeAPIReady();
    return playFunc;
  }

Paul_Wilkins · June 24, 2022, 1:22am

I don’t know. I’m unable to investigate at this stage.

asasass · June 24, 2022, 2:05am

What should I do starting from the beginning with all passing tests?

https://jsfiddle.net/rjna71sy/1/

First we will be working on removing iframe = stubYT(); from the //cleanup section.

      // cleanup
      iframe = stubYT();
      videoPlayer.init(["0dgNc5S8cLI"]);  
    });

Doing this: https://jsfiddle.net/L2e1sfcg/1/

Causes this error:

TypeError: Cannot read properties of undefined (reading ‘nodeName’)

  beforeAll(function() {
    //iframe = stubYT();

  function initVideoPlayer(videoIds = ["0dgNc5S8cLI"]) {
  iframe = stubYT();

After removing the cleanup section we can do this next.

Afterwards we can work on improving the init function so that it behaves appropriately when given no arguments.

Paul_Wilkins · June 24, 2022, 5:51am

In the four part process of dealing with the play button issue:

Test the problem with tests failing
Test the problem with tests passing
Test the solution with tests failing
Test the solution with tests passing

We are part-way through part 2, but not finished with part 2 until refactoring has been done.

Anything relating to improving the init function or other things can not be dealt with until after we have fully completed the above steps for the play issue.

From the investigation in post #1714 it was concluded that an appropriate structure involved stubYT, init, and the ready handler.

Currently stubYT() is called in the beforeAll section. That we now know is not such a good place for it due to changes that can occur, so moving the stubYT() call so that it’s instead the first thing inside the initVideoPlayer() function, is a good start to things.

asasass · June 24, 2022, 1:44pm

I did that here which causes an error.

https://jsfiddle.net/L2e1sfcg/1/

TypeError: Cannot read properties of undefined (reading ‘nodeName’)

  function initVideoPlayer(videoIds = ["0dgNc5S8cLI"]) {
  iframe = stubYT();

This we do after refactoring?

Afterwards we can work on improving the init function so that it behaves appropriately when given no arguments.

Anything relating to improving the init function or other things can not be dealt with until after we have fully completed the above steps for the play issue.

That would be a yes then.

Paul_Wilkins · June 24, 2022, 8:26pm

I’m not getting any error at all from that.

Can you please give more details about the error and your browser version?

asasass · June 24, 2022, 9:04pm

Chrome, have to press Run a million times.

Took me Clicking Run 25 times.

Does this info help?

Paul_Wilkins · June 24, 2022, 9:33pm

Yes that helps thanks. Knowing that it is the “shouldn’t” test that’s failing, I can force the test and get it to always fail when it’s the first of the videoPlayer tests to run.

The issue here is that the iframe variable hasn’t yet been defined. We can fix that by giving the resetPlayer function a suitable iframe element.

        return {
          e: document.createElement("iframe")
        };

Topic		Replies	Views
Adding tests to video player code JavaScript	263	19191	January 7, 2022
Adding tests to video player code JavaScript	18	4169	September 5, 2021
Why is playVideo() not in the code? JavaScript	118	10679	August 19, 2021
Questions about the spinner code JavaScript	41	3683	February 25, 2022
Preventing the browser from reading the YouTube code until the image was clicked JavaScript	564	20608	December 1, 2018

Setting up single-player tests before adding spinner

Related topics