Week-long test coverage montage

• Right into the danger zone
• Risin’ up to the challenge of our rival
• Oh darlin’ my heart’s on fire
• Lose your blues, everybody cut footloose

Key takeaways

beforeEach does not reset any spy that it sets. Will need to find another approach if this is needed. I think setting testObj from a copy of the target object, then spying on that is enough to avoid problems, but there are instances where spy is being set on the nested test objects directly, which will likely affect the tests for those objects at some point.

Maybe this is true for all testing, not just JS, but it looks like even if you have only the happy path, the lines count as covered. Just having coverage is not enough; the tests have to make sense without the coverage. You could have the worst unit tests in the history of unit tests and still have 100% coverage, just by invoking all the paths. This would tell you the paths execute, but not that they yield the correct values. This is somewhat frustrating, because the coverage tool can be a misleading indicator of quality.

Test coverage is excellent for calling out unneccessary or dead code. Having combed through each line of the source, I discovered several bits that were completely unnecessary.

Clever heading (2018-03-18)

Commits

• fix isMobile call test
• correct failing tests; add a few more to game-spec

Today I’m still troubleshooting the Karma errors.

(Sidenote: finally got around to adding a rectangle-select screenshot tool: Shutter, like Greenshot in Windows. It doesn’t have built-in key bindings, but here’s how to configure them. Gird yourselves)

Having a great time trying to troubleshoot why this isn’t working:

1

gameTestObj = Object.assign({}, game);

It SHOULD work, and it should show me a game with these properties:

1
2
3
4
5
6

var game = {
  paused : true,
  levelOver : false,
  gameOver : false,
  delayed : 0,
  delayEndTime : 300,

But all I get is:

It’s missing all of the default params: paused, levelOver, etc.

Turns out, this is because I’m overriding game in interval-creatures-spec!

1
2
3
4
5
6
7

describe('Testing intervalCreatures functions', () => {
  beforeEach(function () {
    testSupporting = Object.assign({}, supporting);
    testObj = Object.assign({}, intervalCreatures);
    testObj.init();
    game = {'gameArea' : {'frameNo' : 10}};
    game.gameArea.canvas = {height: 800, width: 800};

So game loaded when Karma started, then got overwritten by this test. If game-spec had run first, it would have behaved differently, and interval-creatures-spec still would have passed. Why doesn’t Jasmine clean up after itself? Why does it allow overwriting of source objects on the fly? I would expect the test to create a copy, use that, then put it back the way it was.

I also figured out how to return values from a spy.

1

spyOn(player.gamePiece, 'crashWith').and.returnValue(true);

Coverage Report! Once again, we meet at last, for the first time

There you are, passing tests. And I made you a few friends.

Now that tests are passing, I can finally get to the coverage report.

On first loading it from file:///path/to/project/coverage/Chrome%2055.0.2883%20(Linux%200.0.0)/index.html, I see 100% coverage on 0 lines of code. What gives?

Ah, I don’t want to test *.js files at the root of the project for the coverage processor, because that’s not where my files are.

Default setting:

1

preprocessors: { '*.js': ['coverage'] }

Correct setting for project:

1

preprocessors: { 'app/scripts/*.js': ['coverage'], 'app/scripts/components/*.js': ['coverage'] }

Woo.

We can also click on the path link to see details for the files in each subdirectory:

Managing expectations (2018-03-19)

Commits

• add test coverage
• add coverage report to karma config; correct getRandom tests in interval-creature-spec

This made me chuckle:

1

Spec 'Testing collision functions checkLaser sets laser.remove to true on impact' has no expectations.

The spec has been thoroughly demoralized by life. Seems the bar is pretty low there.

Clever heading (2018-03-20)

Commits

• add tests; break up updateGameState in main.js

Ran into a snag when adding a bunch of tests all at once, with an error claiming a function was not defined, but it clearly was. It turns out I made the mistake of adding the tests outside the describe section, again.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10

describe('Testing lasers functions', () => {
  function theTestFunction() {
    // do thing
  };
};

// can't access theTestFunction()
it('calls theTestFunction', () => {
  theTestFunction();
});

I also found another issue with object inheritance: lasers is created from displayObjectPrototype with non-writable functions, preventing Jasmine from spying on those functions.

1

Error: <spyOn> : clear is not declared writable or has no setter.

I set them to writable for now, but there’s probably a better way to handle this, because if they’re not meant to be writable, setting them so just to be able to test means there’s probably a better way.

Coverage is looking better. Attempting to add tests for main.js made me refactor the behemoth updateGameState function (updateGameState is down to 8 lines from 43), which made me take a closer look at how I was handling the player death states. Now all player death management is in a single if block. This should be pulled out further into its own function. I’d like to keep it in main, since putting it in player would add a dependency on game to player.

Now (1 branch, 4 delegations; processTriggers calls checkPlayerDied):

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37

function updateGameState() {
  detectGamePad();
  if (processTriggers()) {
    return;
  };
  prepTheCanvas();
  manageGameObjects();
};

function processTriggers() {
  let triggered =
    checkPlayerDied() ||
    checkLevelOver() ||
    checkGameOver() ||
    checkPause()
  ;
  return triggered;
};

function checkPlayerDied() {
  if (player.died) {
    if (game.delayed === 0) {
      game.setDiedText();
      game.playDiedSound();
      game.delayed++;
      return true;
    } else if (game.delayed < game.delayEndTime) {
      game.delayed++;
      return true;
    } else {
      game.delayed = 0;
      game.manageDeath();
      return true;
    };
  };
  return false;
};

Before (too much to reasonably test):

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43

function updateGameState() {
  // this gets executed every interval
  // check game conditions and update messages
  game.manageGameOver();
  controls.checkControllerState();
  controls.handleGamePause();
  if (game.paused) {
    game.managePause();
    return;
  };
  if (player.died && game.delayed < game.delayEndTime) {
    game.delayed++;
    return;
  };
  if (player.died) {
    game.manageDeath();
    game.delayed = 0;
    return;
  };
  // clear the canvas
  game.checkLevelEndConditions();
  game.startNextFrame();
  manageSounds();
  hud.update();
  // make things happen
  mushrooms.manage();
  centipedes.manage();
  intervalCreatures.manage();
  spiders.manage();
  lasers.manage();
  player.manage();
  // check game conditions
  collisions.check();
  metrics.updateFloatingPoints();
  if (player.died) {
    game.setDiedText();
    game.playDiedSound();
    return;
  };
  if (game.levelOver) {
    game.manageLevel();
  }
};

Coverage:

Clever heading (2018-03-21)

Commits

• knobs-and-levers was getting altered, so texts tests were failing
• add init in several places
• test main.js delegate functions
• add tests for processTriggers delegate functions
• add dom specifications
• add sound specs; consolidate knobsAndLevers specs

I had several issues with canvas parameters not being available. I had to add knobsAndLevers.init() and game.init() in several places. Is this because spies on these objects STAY on the objects after the tests are done? How to resolve that? Should I have to re-init them after every test? Probably. Just irritating that they aren’t automatically re-loaded after each test, and that spyOn permanently alters the object. Still struggling with this.

On further thought, I don’t think that’s what’s happening at all. Instead, I think in some scenarios an error did not occur because the target object was initialized by a test run previously, and tests are run in a random order, so sometimes that test did not run previously.

Added delegate functions to processTriggers in main.js.

Before:

After:

Added dom-spec.js:

Before:

After:

Added sound-spec.js:

Before:

After:

Clever heading (2018-03-22)

Commits

• add collisions test coverage
• add sound test coverage

More test coverage. Removed a lot of unnecessary code from collisions-spec.js. That’s my last yellow bar in scripts/; currently 4% away from green.

(Time passes) Green enough!

Before:

After:

If I click on the file name, I can see exactly what is missed. The E here shows a missed else branch:

The red here shows missed function calls (41, 44) and missed statements (42, 45):

Some notes on the highlighting colors.

Clever heading (2018-03-24)

Commits

• add spider and text tests
• add coverage for mushrooms
• add coverage for metrics
• add display-object-prototype tests

Added more coverage.

One problem I ran into was trying to refactor AS I was writing tests, without actually running the game. intervalCreatures might be a little more complicated than it needs to be; this is where I ran into problems. I’m not sure the constructorFunctions pattern makes sense.

scripts/components are now about 60%; scripts/ are about 92%.

Clever heading (2018-03-25)

Commits

• add lasers tests
• fix flies and worms spawning
• fix initialization errors
• add centipede tests
• add player tests
• add function and branch coverage
• finish off outstanding coverage tests
• reset default parameters
• correct tests

I removed several code bits that were completely unnecessary.

For example, the centipede interval turned out to be meaningless, because the centipede segment is not pushed into the array if it collides with an existing segment, and because the call to spawn is not processed when max number of segments is reached. This means the numberSpawned check could be moved up from spawn into eligibleToSpawn, and the interval can be removed entirely.

I’m finally satisfied with coverage; game-area.js could be better, but we can get that later.