Setting up single-player tests before adding spinner

Paul_Wilkins · January 10, 2022, 9:37pm

No, that doesn’t get done.

☒ Fail ☐ Pass ☐ Refactor

From the code at https://jsfiddle.net/0zqx6ft3/2/ we are looking for a way to have a failing test, when manageCover.init() is run, to become a passing test.

That means looking for something that the init() code changes, that we can test for.

The easiest kind of test is when the function returns something. That doesn’t happen here.

The next easiest is when the function changes something, which is commonly called a side-effect. Does any of that happen in this init() code?

  function init(callback) {
    const cover = document.querySelector(".play");
    cover.addEventListener("click", coverClickHandler);
    events.afterClickCover = new Event("afterClickCover");
    cover.addEventListener("afterClickCover", callback);
  }

The first two lines of the function make no changes that we can directly test for from outside of the function.

The last two lines of the function involve the callback which we aren’t dealing with yet, so we can ignore those for now.

There are no direct changes that occur, so we must move on to indirect changes. That’s where we use the click event from the first two lines of code.

What does the coverClickHandler do? Here it is:

  function coverClickHandler(evt) {
    const cover = evt.currentTarget;
    const curtain = openCurtain(cover);
    showVideo(curtain);
    cover.dispatchEvent(events.afterClickCover);
  }

The openCurtain() function possibly does things that we can test, as well as the showVideo() function. That showVideo() function is what we call “going for the gold” and needs to be held off for as long as possible. If we most directly test that then we won’t be as interested in testing other things around it that need testing, so delayed gratification really is needed here.

In that openCurtain() function is there anything that we can test for?

  function openCurtain(cover) {
    hide(cover);
    const curtain = document.querySelector(".curtain");
    curtain.classList.add("slide");
    return curtain;
  }

There are two things that we can test for, the hiding of the cover and adding slide to the curtain. Those will be two separate tests.

With hiding the cover, we need to ensure that it is first showing before the test calls manageCover.init(), so that we can expect that it is not showing afterwards.

So in the test, before manageCover.init(), define a cover variable the same as what’s done on line 32, that being the first line of the init() function.

We then use classList to remove “hide” from the cover.

After that the manageCover.init() gets run, and after that we need to expect that the cover doesn’t doesn’t have the class “hide”, by using the toHaveClass() matcher.

And after doing that we should end up with a suitably failing test, from which we can then work on making that test pass.

asasass · January 10, 2022, 10:20pm

So in the test, before manageCover.init(), define a cover variable the same as what’s done on line 32, that being the first line of the init() function.

I did that here: https://jsfiddle.net/o1qwh73L/

  it("with no parameters", function() {
    const cover = document.querySelector(".play");
    cover.classList.remove("hide");
    manageCover.init();
  });
});

After that the manageCover.init() gets run, and after that we need to expect that the cover doesn’t doesn’t have the class “hide”, by using the toHaveClass() matcher.

Where exactly is this line being placed?

expect(cover).toHaveClass("hide");

Like this? https://jsfiddle.net/o1qwh73L/1/

it("with no parameters", function() {
    const cover = document.querySelector(".play");
    cover.classList.remove("hide");
    manageCover.init();
    expect(cover).toHaveClass("hide");
  });
});

After that the manageCover.init() gets run, and after that we need to expect

So, that’s why I placed expect after manageCover.init(); If I did that right.

Here is the failing test:

Expected <button class="play" type="button" aria-label="Open"> to have class 'hide'.

Paul_Wilkins · January 10, 2022, 11:15pm

🗹 Fail ☐ Pass ☐ Refactor

Good one. The test is now set up so that when we trigger something that manageCover.init() set up, the test will pass.

🗹 Fail ☒ Pass ☐ Refactor

In this case, just before the expect line we can call simulateClick() with cover as the function parameter, and the test should pass.

We can then rename the test to something more suitable. A longer-winded description of what is happening is that clicking on the cover causes the cover to be hidden. A shorter description that’s suitable for updating the test description is: “initialized cover hides when clicked”

A reason why that is a good test name is that it naturally leads us to the next test, where “uninitialized cover doesn’t hide when clicked”

asasass · January 10, 2022, 11:31pm

This is what I have: https://jsfiddle.net/jrL7qazn/1/

  it("initialized cover hides when clicked", function() {
    const cover = document.querySelector(".play");
    cover.classList.remove("hide");
    manageCover.init();
    simulateClick(cover);
    expect(cover).toHaveClass("hide");
  });

  it("uninitialized cover doesn’t hide when clicked", function() {

  });

Are these the 2 lines that are being changed in the next it section?
https://jsfiddle.net/oz8j4rqm/3/

  it("uninitialized cover doesn’t hide when clicked", function() {
    const cover = document.querySelector(".play");
    // cover.classList.remove("hide");
    manageCover.init();
    simulateClick(cover);
    // expect(cover).toHaveClass("hide");
  });
});

We know that:

hide(cover);

Refers to the play button.

//hide(cover);

When that line is removed, the play button stays visible on the screen after it is clicked.

Working Code: https://jsfiddle.net/0tkLw1d5/

Paul_Wilkins · January 11, 2022, 1:05am

☒ Fail ☐ Pass ☐ Refactor

The uninitialized test shouldn’t have manageCover.init(), and should have everything else the same as the previous test, including the simulateClick().

Because manageCover.init() is used to let us to change the hidden cover to being a shown cover, when manageCover.init() is not used, the cover should still remain hidden when we click on it.

Other than having no manageCover.init(), the only other change is that the expectation is reversed, expecting cover not to have that class.

You should then find that the test is unreliable, sometimes passing and sometimes failing, depending on the random order in which the tests is run. It’s good that we found that out now before we had other tests to add confusion to the troubleshooting, and it’s good that we have random test orders that help to expose such issues for us.

The unreliable tests are because the cover is not reliably in the same state when the test runs. Sometimes it has a click handler from a previous init, and sometimes it doesn’t. We will take care of next, but not directly with the afterEach function.

asasass · January 11, 2022, 1:09am

This is what I have: https://jsfiddle.net/0zp6emkc/1/

  it("initialized cover hides when clicked", function() {
    const cover = document.querySelector(".play");
    cover.classList.remove("hide");
    manageCover.init();
    simulateClick(cover);
    expect(cover).toHaveClass("hide");
  });

  it("uninitialized cover doesn’t hide when clicked", function() {
    const cover = document.querySelector(".play");
    cover.classList.remove("hide");
    simulateClick(cover);
    expect(cover).not.toHaveClass("hide");
  });
});

What I will be adding next will be placed under the describe section?

describe("init manageCover", function() {

Paul_Wilkins · January 11, 2022, 3:20am

🗹 Fail ☒ Pass ☐ Refactor

Yes, that will be a function called removeEventHandlers and has a function parameter of el. Inside of that function, the innerHTML happens to the parentNode of that el variable instead, similar to the afterEach function.

It is inside of a separate function called afterEach is where we define the cover variable, and pass that cover as an argument to the removeEventHandlers function. The whole intention here is that instead of having mysterious innerHTML stuff happening, the function name of removeEventHandlers helps to inform us about what is going on there.

After getting that working, we will have some further work to do on the cover variable afterwards.

asasass · January 11, 2022, 3:32am

This is what I have:

  function removeEventHandlers(el) {
    afterEach(function(el) {
      const cover = document.querySelector(removeEventHandlers);
      cover.innerHTML = cover.innerHTML;
    });
  }

Paul_Wilkins · January 11, 2022, 3:51am

🗹 Fail ☒ Pass ☐ Refactor

asasass:

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

  function removeEventHandlers(el) {
    function afterEach(el) {
      const cover = document.querySelector(removeEventHandlers);
      cover.innerHTML = cover.innerHTML;
    }
  }

That starts of being what you were asked to do but then becomes a mess.

The afterEach function needs to be separate from the removeEventHandlers() function.
Then the removeEventHandlers() function needs to do what you were asked to do with it.
Then, the afterEach() function needs to do what you were asked to do with it.

asasass · January 11, 2022, 3:52am

I’m having a hard time with this.

This is all confusing to me:

I can’t do this all at 1 time.

Some pieces of code I have more experience working with than others.

innerHTML afterEach is fairly new to me.

Yes, that will be a function called removeEventHandlers and has a function parameter of el. Inside of that function, the innerHTML happens to the parentNode of that el variable instead, similar to the afterEach function.

It is inside of a separate function called afterEach is where we define the cover variable, and pass that cover as an argument to the removeEventHandlers function. The whole intention here is that instead of having mysterious innerHTML stuff happening, the function name of removeEventHandlers helps to inform us about what is going on there.

After getting that working, we will have some further work to do on the cover variable afterwards.

First this:

The afterEach function needs to be separate from the removeEventHandlers() function.

Is this what you mean by separate?

  function removeEventHandlers(el) {

  }

  afterEach(function(el) {

  });

I don’t understand this:

the innerHTML happens to the parentNode of that el variable instead, similar to the afterEach function.

This means:

where we define the cover variable, and pass that cover as an argument to the removeEventHandlers function

I do this? const cover = document.querySelector(removeEventHandlers);

Is this being used in the code? cover.innerHTML = cover.innerHTML;

Paul_Wilkins · January 11, 2022, 5:35am

🗹 Fail ☒ Pass ☐ Refactor

Let’s try this in a different order.

From the code at https://jsfiddle.net/0zp6emkc/1/

Add an empty afterEach section above the simulateClick function.
In that afterEach section, make a function call to the not-yet-existing removeEventHandlers() function.
Add an argument of cover to that removeEventHandlers() function call.
When you run the test it then says that the removeEventHandlers() function doesn’t exist, so create that function above the afterEach section.
The test then says that cover isn’t defined, so define the cover variable in the afterEach section the same way that we’ve defined it elsewhere. Yes that is duplication of the cover variable, and yes we will remove that duplication soon.

The tests go back to being erratic, sometimes working and sometimes not, because we need to finish the removeEventHandlers() function.

Add a function parameter of el to the removeEventHandlers function.
Inside of that function assign el.outerHTML to equal el.outerHTML

Using outerHTML removes the need for using parentNode if we had used innerHTML.
That should be all that you need there to get things working.

When the removeEventHandlers() function is being used to make the tests behave in a reliable manner, we can then move on to tidying up some duplication of the cover variable.

asasass · January 11, 2022, 9:24am

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

Is this good?

  function removeEventHandlers(el) {

    el.outerHTML = el.outerHTML;

    afterEach(function(cover) {
      removeEventHandlers(cover);
    });
  }

Paul_Wilkins · January 11, 2022, 9:27am

For god’s sake man - stop putting the afterEach section inside of the removeEventHandlers function!

Separate those things!

asasass · January 11, 2022, 4:00pm

define the cover variable in the afterEach section the same way that we’ve defined it elsewhere

Would mean this right? https://jsfiddle.net/t5bnfug9/

Am I able to make progress from here?

If this is good, what am I up to next?

  function removeEventHandlers(el) {
    el.outerHTML = el.outerHTML;
  }

  afterEach(function() {
    const cover = document.querySelector(".play");
    removeEventHandlers(cover);
  });

Paul_Wilkins · January 11, 2022, 9:32pm

🗹 Fail 🗹 Pass ☐ Refactor
The test is now suitably passing, and we can move on to removing duplication of the cover variable now.

🗹 Fail 🗹 Pass ☒ Refactor

Currently in the code at https://jsfiddle.net/t5bnfug9/ you will see that we are using const cover in the afterEach section, and in each of the tests. repetition will continue to grow as we add more tests. We want to avoid that repetition.

Just below the “init manageCover” line we can use let to define a cover variable. Just define the variable, don’t even use the equals sign there. We can’t assign it here because the afterEach section changes things, resulting in it needing to be reassigned. We do that in the beforeEach section instead.

Below the let cover line add a beforeEach section, similar in design to the afterEach section. In that beforeEach section we can assign the cover variable, in the same way as has been done elsewhere, but without the const variable.

All of the const cover lines can now be removed from the testing code.

asasass · January 11, 2022, 10:02pm

I did that here and the code passes: https://jsfiddle.net/5vofhyc8/3/

What am I up to next?

describe("init manageCover", function() {

  let cover;

  beforeEach(function() {
    cover = document.querySelector(".play");
  });

  function removeEventHandlers(el) {
    el.outerHTML = el.outerHTML;
  }

  afterEach(function() {
    cover = document.querySelector(".play");
    removeEventHandlers(cover);
  });

  function simulateClick(el) {
    const clickEvent = new MouseEvent('click', {
      currentTarget: 'el'
    });
    el.dispatchEvent(clickEvent);
  }

  it("initialized cover hides when clicked", function() {
    cover.classList.remove("hide");
    manageCover.init();
    simulateClick(cover);
    expect(cover).toHaveClass("hide");
  });

  it("uninitialized cover doesn’t hide when clicked", function() {
    cover.classList.remove("hide");
    simulateClick(cover);
    expect(cover).not.toHaveClass("hide");
  });
});

Paul_Wilkins · January 11, 2022, 11:12pm

🗹 Fail 🗹 Pass ☒ Refactor

We can now update the initialized test before moving on to other tests.

Move the existing initialized test below the uninitialized test. All of the other tests we will be doing will be for initialized code, so remove initialized from the test description.

Also, move the manageCover.init line to the top of the test that it’s in. That way it becomes more clear that removing hide is not for the benefit of manageCover.init, but for the simulateClick instead.

asasass · January 11, 2022, 11:25pm

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

describe("init manageCover", function() {
  
  let cover;

  beforeEach(function() {
    cover = document.querySelector(".play");
  });

  function removeEventHandlers(el) {
    el.outerHTML = el.outerHTML;
  }

  afterEach(function() {
    cover = document.querySelector(".play");
    removeEventHandlers(cover);
  });

  function simulateClick(el) {
    const clickEvent = new MouseEvent('click', {
      currentTarget: 'el'
    });
    el.dispatchEvent(clickEvent);
  }

  it("uninitialized cover doesn’t hide when clicked", function() {
    cover.classList.remove("hide");
    simulateClick(cover);
    expect(cover).not.toHaveClass("hide");
  });
  
    it("cover hides when clicked", function() {
    manageCover.init();
    cover.classList.remove("hide");
    simulateClick(cover);
    expect(cover).toHaveClass("hide");
  });
});

Paul_Wilkins · January 12, 2022, 12:33am

🗹 Fail 🗹 Pass ☒ Refactor

Remove that cover assignment from the afterEach code. There’s no reason for that to be there at all.

The removeEventHandlers() function can now be moved up above where the cover variable is defined, and the simulateClick() function can be moved up above that removeEventHandlers() function.

That way things are nicely organised, starting with the support functions, then the cover variable is defined, which is used by the beforeEach section, which is followed by the afterEach section, and they are followed by the it tests themself.

That cover-hides description should also be slightly renamed so that it makes more sense. Renaming the description to “hides the cover when clicked” helps to make it read better.

As a recap, before each test we are gaining a reference to the cover element, which each test can use if it wants. After each test we are cleaning up by removing any events that may be on the cover.

asasass · January 12, 2022, 1:09am

Here is what I have: https://jsfiddle.net/bmn27azt/

Am I ready to add a new test?

describe("init manageCover", function() {

  function simulateClick(el) {
    const clickEvent = new MouseEvent('click', {
      currentTarget: 'el'
    });
    el.dispatchEvent(clickEvent);
  }

  function removeEventHandlers(el) {
    el.outerHTML = el.outerHTML;
  }

  let cover;

  beforeEach(function() {
    cover = document.querySelector(".play");
  });

  afterEach(function() {
    removeEventHandlers(cover);
  });

  it("uninitialized cover doesn’t hide when clicked", function() {
    cover.classList.remove("hide");
    simulateClick(cover);
    expect(cover).not.toHaveClass("hide");
  });

  it("hides the cover when clicked", function() {
    manageCover.init();
    cover.classList.remove("hide");
    simulateClick(cover);
    expect(cover).toHaveClass("hide");
  });
});