Adding tests to video player code

You wanted me to do this? https://jsfiddle.net/10gncosb/

It’s passing here.

describe("init", function() {
  it("with no parameters", function() {
    const fnCall = () => manageCover.init();
    expect(fnCall).toThrow();
  });
});

Yes. The manageCover.init() is still throwing an error, but the test now knows that we expect it to have an error.

A better name for this it() section to replace “with no parameters” is “needs a selector”

Inside of the describe function we will have several it() sections, one after another, that each check for a small and simple thing that is expected to occur.

I did this: https://jsfiddle.net/n4gd7e6k/

describe("init", function() {
  it("needs a selector", function() {
    const fnCall = () => manageCover.init();
    expect(fnCall).toThrow();
  });
});

Inside of the describe function we will have several it() sections, one after another

Like this: https://jsfiddle.net/n4gd7e6k/1/

describe("init", function() {
  it("needs a selector", function() {
    const fnCall = () => manageCover.init();
    expect(fnCall).toThrow();
  });
  
    it("needs a selector", function() {
    const fnCall = () => manageCover.init();
    expect(fnCall).toThrow();
  });
  
  
    it("needs a selector", function() {
    const fnCall = () => manageCover.init();
    expect(fnCall).toThrow();
  });
  
    it("needs a selector", function() {
    const fnCall = () => manageCover.init();
    expect(fnCall).toThrow();
  });
  
  
});

Then we will match these tests up with the broken code?

Yes, like that. For now can you please delete the extra ones so that they don’t end up confusing everything.

The init function uses two object properties, one called container and one called playButton. We will be next testing what is expected to happen with the container property.

We need a new test where we give the init method a container property, that has the “.play1” selector.

About the only thing that we can check in regard to that selector is when it is shown or hidden.
The manageCover code does hide the container when the animationend event occurs, so we need to use querySelector to get “.play1” container, so that we can remove the “hide” class from it.

After that we will expect that the container has the “hide” class, which should suitably fail for us.

After that, and just before the expect, we can trigger the animationend event which will hide the container, causing the test to properly pass.

I don’t understand what I am supposed to do. https://jsfiddle.net/2huvxtjm/

This was one test, we need to make another one of these?

This one gets removed or changed to a different test?

describe("init", function() {
  it("needs a selector", function() {
    const fnCall = () => manageCover.init();
    expect(fnCall).toThrow();
  });
});


Yes we do. Make another it section, and in there call the init method of manageCover.
When you’ve done that that init method a function parameter of an object, where the only property on that object is container: "play1"

This is what I have: https://jsfiddle.net/oj8fb4v2/

Is this what you meant by make another it section?

describe("init", function() {
  it("needs a selector", function() {
    const fnCall = () => manageCover.init();
    expect(fnCall).toThrow();
  });

  it("needs a selector", function() {
    const fnCall = () => manageCover.init( 
    container: "play1"
    );
  });
});

The title of the second test shouldn’t be the same as any of the other tests in that area. With this test we are checking that it hides the container on animation end, so we should call this this “hides the container on animation end”

There should be no const line, that fnCall thing was unique to the previous test.

The manageCover line should be more like in the post post #19 with the container property inside of the parenthesis.

Currently the container property is not valid, because you are missing the curly braces to indicate that it’s an object. Add an opening curly brace after the starting function parameter parenthesis, and a closing curly brace before the closing function parameter parenthesis to fix that.

Also, “play1” is not a suitable selector, it’s missing a fullstop at the start.

Keep it up though, we’re making some progress.

I don’t understand what you mean by full stop.

This is what I have: https://jsfiddle.net/6d5wqz72/3/

describe("init", function() {
  it("with no parameters", function() {
    manageCover.init(
      container; {
        "play1",
        {}
      });
  });
});

Please don’t remove the “needs a selector” test. That was good and should remain as it was.

/me looks at the code link

Ahh, I see what you’ve done. You have two separate describe sections for init. There should only be one describe section for init.

Please use only the one describe section by combining them together so that you have two it sections inside of the one describe section.

This is what I have: https://jsfiddle.net/61gv0xy5/

describe("init", function() {
  it("needs a selector", function() {
    const fnCall = () => manageCover.init();
    expect(fnCall).toThrow();
  });

  it("with no parameters", function() {
    manageCover.init(
      container; {
        "play1",
        {}
      });
  });

});

Can you please just do an object inside of init without screwing up the object structure?

Here is an example of how an object is structured:

And here’s some feedback about what needs to be fixed with you attempt at an object.

  it("with no parameters", function() {
    manageCover.init( // should have an opening curly brace after the opening parenthesis
      container; { // semicolon should be a colon, the curly brace needs to be removed
        "play1", // should be on the same line as container, is still missing the fullstop class selector
        {} // this line serves no benefit or purpose here at all and must be removed
      });
  });

Is this good?

This is what I have: https://jsfiddle.net/efwmjax7/4/

  it("with no parameters", function() {
    manageCover.init({
      container: {"play1":{}}
    });
  });
});

I was told that, in my showCover function I am referencing the parent object of the play button. which I changed from a div with the class of container play1 with-curtain to a div with the class playButtonContainer. And I may have some other code that does not like that change.

That may be some useful information to figure out what the issue is and what is needed to fix it.

 function showCover(playButton) {
    hideAll(config.containers);
    resetPage();
    markAsPlayed(playButton);
    const cover = playButton.parentElement;
    cover.classList.add("active");
    show(cover);
  }

And as simple as that, the tests that form the documentation for your code will never be done.

I’ll let you know if I figure out a simple way to guide you through the advice you’ve been given.

Will that advice be able to help guide a way to resolve/fix the issue?

I tried adding a console log statement to the broken and working code, but that is not telling me much, unless I am doing it wrong, or maybe more than one is needed.

Both tell me the same thing.

I ran both of these through code beautifiers so they are both neat.
I checked for jslint errors and neither of them have any.
And there are no console log errors appearing in either also.

Broken Code: https://jsfiddle.net/dbh3wymv/

Working Code: https://jsfiddle.net/4b1nj28o/

CSS

.playButtonContainer {
  display: flex;
  flex-wrap: wrap;
  min-height: 100%;
  margin: auto;
  justify-content: center;
  align-content: center;
  width: 290px;
  gap: 10px;
  animation: fadeInButtons 3s ease 0s forwards;
}

html

<div class="outer">
  <div class="container play1 with-curtain">
    <div class="inner-container curtain curtain1">
        <div class="wrap">
          <div class="video video-frame" data-id="CHahce95B1g"></div>
        </div>
      <button class="exit" type="button" title="Exit" aria-label="Close"></button>
    </div>
  </div>
</div>

<div class="playButtonContainer">
  <button class="playa cover" type="button" aria-label="Open"></button>
</div>

Javascript

function showCover(playButton) {
    hideAll(config.containers);
    resetPage();
    markAsPlayed(playButton);
    const cover = playButton.parentElement;
    cover.classList.add("active");
    show(cover);
    console.log(showCover);
  }

Can we complete these tests then for manageCover ?

My intention was to complete the tests, but then I got information about what the issue is.

Well, what the tests were for was to find out the reason why something isn’t working because we didn’t know.

But I think now we know why it is not working.

When new information comes to light, reassess, then decide how to move forward.

With that new information,

My thinking was to do this, maybe I am close, not sure.

But then, would manageCover need to be called something else, I’m not sure.

And maybe I am wrong here and this wouldn’t be needed, not necessary, or it is not how the issue would be fixed.

const managePlayButtons = (function makePlayContainer() {
const manageCoverButtons = (function makeCoverContainer() {

  return {
  };
}());

Then, below that this:

const manageCover = (function makeManageCover() {

  return {
  };
}());

or, maybe set like this:

This: playButtonContainer, or just, playContainer

Would refer to this:

const manageCover = (function makeManageCover() {

  return {
  };
}());

This: <div class="container play1 with-curtain">

Would refer to this:

const manageContainer = (function makeManageContainer() {

  return {
  };
}());

Which would mean changing:
const manageCover = (function makeManageCover(){
to: const manageContainer = (function makeManageContainer() {

This is already done in the code:

This <div class="inner-container curtain curtain1">

Refers to this:
const manageUI = (function makeManageUI() {

The flow UML would look like this:

The only new one that is being added is the playContainer code

playContainer const manageCover = (function makeManageCover() {
class=“container play1 with-curtain” const manageContainer = (function makeManageContainer() {

The original manageCover, the name would be changed to: manageContainer

const manageUI = (function makeManageUI() {
const videoPlayer = (function makeVideoPlayer() {
const managePlayer = (function makeManagePlayer() {
const players = (function coverContainerUIPlayerFacade() {

And maybe all of those changes I thought of just now are not necessary.

I am not really sure what the next steps would be here.

First come up with a plan on what is going to be done.
Then, start to implement it.

I am able to come up with all these ideas on ways it might be able to work, but I don’t know what is the right way to do this.

The right way is to document the current expected behavior of manageCover, using tests, so that we can use those tests can help us to find the bad behavior in the non working code,

That is how most skilled programmers these days reduce the complexity, so that they can be fast and efficient at all their coding.

2 Likes

Lets do that then.

We would be testing this function:

function showCover(playButton) {

I am trying to make progress.

The bad behavior was told to me here: at post #34

Should we try to confirm that through tests?

or through console log statements?

How can we confirm the bad behavior that was told to me?

Assuming that is what the bad behavior is.

It can either be confirmed that that is what is bad in the code, or it isn’t.

The next steps here would be?

My last test I did was at post #33

How can we make progress?

What do you want me to do?