Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

expect() with no actual expectations

The Problem:

Recently, while reviewing our existing test codebase, I've noticed a dangerous kind of typo/mistake when expect() was used without the "matching" part:

expect(page.filters.fromDateLabel.getText(), "After");

I'm pretty sure toEqual() was meant to be used here:

expect(page.filters.fromDateLabel.getText()).toEqual("After");

The problem with this is that jasmine would not fail the expectation in this case (well, obviously because nothing was actually expected). And this gets us to a more serious problem - nothing was actually tested in a test case - it was passing with no expectations made. We were getting a false sense of what was tested.

The Question:

I want to catch these mistakes as fast as possible. How do you think I should handle the problem?

Thoughts:

  • somehow fail a test case if there was no expectations made in it (not sure if jasmine has anything like this built-in)
  • "patch" the expect() and issue a warning/raise an error if nothing was called on the "expect" part
  • use static code analysis - define a custom eslint rule
like image 937
alecxe Avatar asked Nov 02 '15 03:11

alecxe


2 Answers

The custom ESLint rule provided in the answer is now a part of eslint-plugin-jasmine 1.6.0:

  • valid-expect

Old Answer:

Here is a custom ESLint rule I've ended up with:

module.exports = function (context) {
  return {
    // checking "expect()" arguments
    CallExpression: function (node) {
      if (node.callee.name === 'expect') {
        if (node.arguments.length > 1) {
          context.report(node, 'More than one argument passed to expect()')
        } else if (node.arguments.length === 0) {
          context.report(node, 'No arguments passed to expect()')
        }
      }
    },

    // nothing called on "expect()"
    'CallExpression:exit': function (node) {
      if (node.callee.name === 'expect' && node.parent.type === 'ExpressionStatement') {
        context.report(node, 'Nothing called on expect()')
      }
    }
  }
}

It checks for 3 things:

  • more than 1 argument passed to expect()
  • no arguments are passed to expect()
  • there was nothing called on expect()

Here are the sample invalid expect() usages it currently catches:

expect(page.filters.fromDateLabel.getText(), "After");
expect("After");
expect();

As for the option #1, there is actually a quite related and useful ESLint rule being already implemented and open-sourced by [eslint-plugin-jasmine]:

  • Enforce expectation (missing-expect)
like image 62
alecxe Avatar answered Oct 17 '22 07:10

alecxe


I tend to think that the static analysis route is best, but if you’re looking for a quick and dirty way, here’s some code that grabs the expectations returned by all calls to expect and creates a proxy that tracks whether any of the expectation’s properties were ever used:

var unusedExpectations = new Set();

var originalExpect = window.expect;  // Should be empty after every spec
var expect = function() {
  var rawExpectation = originalExpect.apply(this, arguments);
  unusedExpectations.add(rawExpectation);  // Assume unused until used

  // Traverse expectation and its prototypes, copying all properties to
  // our proxy object. (Note that this becomes much simpler if you have
  // ES6 Proxy in your environment.)

  var proxy = {}
  for(var proto = rawExpectation; proto; proto = proto.__proto__) {
    Object.getOwnPropertyNames(proto).forEach(function(prop) {
      if(Object.getOwnPropertyDescriptor(proxy, prop))
        return;
      Object.defineProperty(
        proxy, prop, {
          get: function() {
            // Aha! Somebody used this expectation for _something_.
            unusedExpectations.delete(rawExpectation);
            return rawExpectation[prop];
          }
        }
      );
    });
  }
  return proxy;
}

Put that in a place where it hides Jasmine’s expect from your specs, and then:

beforeEach(function() {
  unusedExpectations.clear();
});
afterEach(function() {
  expect(unusedExpectations.size).toEqual(0);
});

Caveats:

  1. Kind of evil.
  2. Will not catch expect(foo).toBeFalsy; (missing parens).
  3. Counts the use of any property, so won’t catch expect(foo).toString().

Still, it works!

One could add code to inspect the stack trace and extract the location of the offending expect(), but I imagine flagging which spec has an unused expect() is sufficient.

like image 1
Paul Cantrell Avatar answered Oct 17 '22 07:10

Paul Cantrell