All Articles

Chai.js & Fake News

A few days ago while writing some tests using Mocha.js and Chai.js, my lovely “red-green-refactor” cycle was halted unexpectedly. A test that should have failed… was passing! As a true test driven development zealot*, my brain treated this as something of a NullPointerException, and I ended up on the floor, having some sort of fit, moving sort of like this:

After picking myself up from the floor, I decided I needed to figure out what had happened. Here’s the tale of my adventure.

I was writing a simple utility function, getRandomArrayItem, so I started by writing a test that looked something like the example below. The menuItems mock represents an Array of Objects that would come from a Couchbase database in production.

import { expect } from 'chai';
import getRandomArrayItem from '../src/utils/getRandomArrayItem';
import mockArray from './mocks/mockArray'

describe('getRandomArrayItem', () => {
  it('exists', () => {
    expect(getRandomArrayItem).to.be.defined;
  });
});

Ok, test failed. That makes sense, because I hadn’t created the module yet. So I did that, and got that lovely green 1 Test Passing message so I added another test:

import { expect } from 'chai';
import getRandomArrayItem from '../src/utils/getRandomArrayItem';
import mockArray from './mocks/mockArray'

describe('getRandomArrayItem', () => {
  it('exists', () => {
    expect(getRandomArrayItem).to.be.defined;
  });
  it('returns something', () => {
    expect(getRandomArrayItem(mockArray)).to.be.defined;
  });
});

As expected, another failure. This time because the module wasn’t returning a function. Next, I added a default export, which was a named (no-op) function, like so:

export default function getRandomArrayItem(mockArray) {}

I ran the tests again expecting to see a failing test… After all, calling getRandomArrayItem(mockArray) should return undefined right? And expect(undefined).to.be.defined couldn’t possibly pass, right?

Nope! And this is the moment things start getting fuzzy. The next thing I really remember is crawling up off of the floor, with the words “Unexpected Arguments” swirling around in my psyche. So, as any person of science would do I decided to perform an experiment to try and understand what had just happened to me. I like to imagine myself in this moment as a much less significant, and probably less profound, Albert Hofmann. So, I added a test just to confirm that I wasn’t in the twilight zone, and came up with this:

  it('returns something', () => {
    expect(getRandomArrayItem(mockArray)).not.to.be.undefined;
  });

After running this and receiving AssertionError: expected undefined not to be undefined, I knew that Mocha and Chai were, at least mostly, okay. I googled a little and couldn’t find any info, so I opened up an issue on the Chai.js repo and soon learned that it’s a known issue, and one they’re discussing a solution for. The issue here is a combination of two things. First, and most glaring, is my syntax problems. I’ve written tests that are not valid.

The problem is that it’s too easy to write tests that appear valid but are actually broken in such a way that they pass no matter what.

@meeber

The assertions in my code are worthless! So, let’s sort that out real quick. Fortunately, it’s an easy change. There’s no .defined assertion, so let’s just make our first test match the third test, since test #3 is reporting accurately. Here’s what our tests look like now:

import { expect } from 'chai';
import getRandomArrayItem from '../src/utils/getRandomArrayItem';
import mockArray from './mocks/mockArray'

describe('getRandomArrayItem', () => {
  it('exists', () => {
    expect(getRandomArrayItem).not.to.be.undefined;
  });
  it('returns something', () => {
    expect(getRandomArrayItem(mockArray)).to.be.defined;
  });
  it('returns something', () => {
    expect(getRandomArrayItem(mockArray)).not.to.be.undefined;
  });
});

This passed, which we should have expected. Let’s work on fixing the one we know is misinforming us. Since the issue is that .defined isn’t a valid assertion, we’ll just change that test to use the valid .not.to.be.undefined. Now our tests look like thi… wait…

describe('getRandomArrayItem', () => {
  it('exists', () => {
    expect(getRandomArrayItem).not.to.be.undefined;
  });
  it('returns something', () => {
    expect(getRandomArrayItem(mockArray)).not.to.be.undefined;
  });
  it('returns something', () => {
    expect(getRandomArrayItem(mockArray)).not.to.be.undefined;
  });
});

Wait a minute. Is it raining in here? Because this doesn’t seem very DRY. Let’s just throw away test #3, since it’s not needed any more. After all, we wrote it just as a sanity check. Now, run the tests and… Failure! Or is it success? I’ll put this whole adventure in the “Fail Forward” column and move along to getting this test to pass.

So now that my tests aren’t fake news, we can talk about what was actually happening under the hood that allowed me to write these worthless tests. It turns out this exact issue is what spawned the creation of dirty-chai, and it’s authors/maintainers have explained the problem on Github. Additionally, the creator of Must.js provides further info on the problem in their repo.

The tl;dr is this: In what I assume was an attempt to make Chai nicer to use, the library was designed so that any assertions that did not require parameters will assert when the property is accessed, rather than requiring the method to be called. While I very much appreciate the effort at making Chai easier to use, this puts me in a position I don’t like to be in… I can’t trust my tests. Going forward, I may start using dirty-chai or I may just port all of my Chai projects to another assertion library. Must.js looks interesting, and the migration is pretty straight forward. I’ve been using Jest from facebook more and more lately though, so maybe I’ll just go that route. Who knows.

-Matt (Not an actual zealot)

Published 9 Apr 2017