Classic Stats Redirect script, now with automated tests!

I released v2.3.0 of the Classic Stats Redirect script on Sunday. Your copy should have automatically updated already. If you don’t already have the script or need to update manually, you can download your copy from Greasy Fork. Special thanks go to Dennis for helping me test out this version!

After the incident causing the release of v2.2.1 (I apparently neglected to announce v2.2.0 and v2.2.1; this release suppresses redirection for URLs containing from=wp-admin), I decided that I should write some automated tests to catch these kinds of errors before I end up releasing them. I will discuss the issue in the technical section that follows.

This update is purely a refactor and the behaviour should be the same. If anything has changed, please leave a comment or file an issue.

Technical Details

This section is purely informational and is not necessary to use the script.

The Problem in v2.2.0

Below is the new bit of code added in v2.2.0 to suppress redirection for URLs containing from=wp-admin.

// redirect unless new stats is explicitly requested
if (!window.location.search.contains("from=wp-admin")) {
  doRedirect(window.location.pathname);
}

To the casual reader, there doesn’t seem to be anything wrong with that. Even when I tested manually, it seemed to function as expected. However, when I pushed the update, Dennis noticed that the script stopped working.

After further investigation, I discovered that String.prototype.contains (note: window.location.search is a String) does not exist; it was renamed to String.prototype.includes due to conflicts with existing libraries. The reason it worked when I was testing is that Pale Moon, the fork of Firefox that I use regularly, implements both contains and includes. On browsers that no longer have contains, attempting to call the non-existent function will throw an exception and halt the execution of the script, meaning no redirect.

v2.2.1 was released to use search instead of contains. search was used over includes because it is older and more widely supported. Looking at it again, perhaps it should use RegExp.prototype.test instead.

The Testing Environment

I decided on running the tests in a Node.js environment instead of the browser. Doing so would allow me to catch errors like the one above, as well as add some easy dependency management.

Mocha is the testing framework I used. I’ve used Mocha a lot for other work-related projects and I have had no complaints about it.

Mocha.js

Chai is used as the assertion library. I chose the assert style over the other styles because I find it’s clearer from a visual code perspective and involves less “magic” and sugar than the other two, which I presume use getters to set up assertions. I’m sure Node’s own assert module is good enough for what I need, but I hadn’t gotten the message that they stopped discouraging its use. I may decide to move to assert at some point.

Chai.js

Sinon is used to stub functions. Stubbing a function involves replacing that function with a dummy function that allows you to inspect the arguments passed in and return whatever value you want.

Sinon.JS

These dependencies are all listed in the project’s new package.json.

Making the Script Testable

In its v2.2.1 form, the script was very difficult to test. The only tests you could do were end-to-end tests where you’d visit a URL and expect to be at another after the script runs. This was what I was already doing manually, but it wouldn’t be good enough for automated tests.

In order to make it testable, I had to encapsulate the existing functions into an object so I could export them. I also abstracted some existing functionality into more functions so that they could be independently tested.

var script = {
  start: function(path) {
    this.doRedirect(this.parseUri(path));
  },

  assembleUrl: function(base, period, query) {
    // omitted
  },

  parseUri: function(uri) {
    // omitted
  },

  doRedirect: function(tokens) {
    // omitted
  },


  utils: {
    // omitted
  }
}

if (typeof module == "object" && module != null) module.exports = script;

if (typeof window == "object"
    && window.location.search.search(/from=wp-admin/) === -1)
  script.start(window.location.pathname);

Line 1 in the code block is the encapsulating object, and line 39 exports it. The check for module at the end is necessary because browsers won’t have this variable defined, and attempting to access the exports property of the undefined variable will throw an exception. This variable only exists by default in a Node environment.

Similarly, window does not exist in Node, so a guard must be included there.

In order to easily stub out “dangerous” code (that is, code that interacts with the browser), I abstracted the functionality into util functions. While, in theory, these functions could be tested, doing so is somewhat more difficult than testing the other functions, and I didn’t find it necessary to test these since they are simple wrappers (except apiFetch, but I got that from Stack Overflow and it isn’t going to change).

// var script = {
  utils: {
    locationReplace: function(url) {
      window.location.replace(url);
    },

    registerOnload: function(onload) {
      window.onload = onload;
    },

    // Based on function by dystroy. From http://stackoverflow.com/a/14388512
    apiFetch: function(path, callback, fallback) {
      var base = "https://public-api.wordpress.com/rest/v1.1";
      var httpRequest = new XMLHttpRequest();
      httpRequest.onreadystatechange = function() {
        if (httpRequest.readyState === 4) {
          if (httpRequest.status === 200) {
            if (callback) callback(JSON.parse(httpRequest.responseText));
          } else if (fallback) {
            fallback();
          }
        }
      };
      httpRequest.open("GET", base + path);
      httpRequest.send();
    }
  }
// }

Since the functions are now encapsulated in an object, referring to them required using this.helperFunction instead of simply helperFunction. This goes for the recursive call in doRedirect as well. For example, the doRedirect function:

// var script = {
  doRedirect: function(tokens) {
    if (tokens.blogDomain) {
      // Redirect to post URL based on API results
      // API docs: https://developer.wordpress.com/docs/api/
      this.utils.apiFetch("/sites/" + tokens.blogDomain,
        // attempt to redirect using API
        (function(data) {
          this.utils.locationReplace(this.assembleUrl(data.URL, tokens.statsType));
        }).bind(this),

        // fallback: attempt to use the blog domain
        (function() {
          // use http instead of https in case the server doesn't support https
          // (e.g. for Jetpack sites)
          this.utils.locationReplace(this.assembleUrl("http://" + tokens.blogDomain, tokens.statsType));
        }).bind(this)
      );
    } else if (tokens.statsType != "insights") {
      this.utils.registerOnload((function() {
        // construct a stats URI from the user's default blog
        var defaultBlogStatsUri = "/stats";
        if (tokens.statsType) defaultBlogStatsUri += "/" + tokens.statsType;
        defaultBlogStatsUri += "/" + currentUser.primarySiteSlug;
        this.doRedirect(this.parseUri(defaultBlogStatsUri));
      }).bind(this));
    } else {
      this.utils.registerOnload((function() {
        // construct an insights URI from the user's default blog
        this.doRedirect(this.parseUri("/stats/insights/" + currentUser.primarySiteSlug));
      }).bind(this));
    }
  },
// }

Another note is the need to bind the value of this for the inner functions. The value of this in a function depends on how the function is called; in this case, the value of this inside the function is not the same as the value of this outside the function. The classical approach is to create a variable called self, thiz or that (or whatever name you want) to hold the value of this from outside the function so that it can be used inside the function. The better way to do it (as of ES5) is to use Function.prototype.bind. ES2015 introduced arrow functions which offers the cleanest solution, but I can’t use ES2015 for compatibility with older browsers (I don’t think anyone is using this script with IE, but you never know).

The Tests

Mocha tests have this general structure:

describe("description of suite", function() {
  beforeEach(function() {
    // setup
  });

  afterEach(function() {
    // teardown
  });

  it("test 1", function() {
    // test 1 actions and assertions
  });

  it("test 2", function() {
    // test 2 actions and assertions
  });
});

it functions are your individual tests. These tests can be grouped with a describe block. describe blocks can be nested to create groups of groups. Setup and teardown code common to all tests within a describe block can be put in the beforeEach and afterEach functions of the block respectively. This is the structure of the test suite:

var sinon = require("sinon");
var assert = require("chai").assert;
var statsRedirect = require("./wpcom-stats-redirect.user.js");

describe("stats redirect", function() {
  var sandbox;

  beforeEach(function() {
    sandbox = sinon.sandbox.create();
  });

  afterEach(function() {
    sandbox.restore();
  });

  describe("start", function() {
    it("should redirect using the parsed path", function() {
      // omitted
    });
  });

  describe("assembleUrl", function() {
    it("should set appropriate unit", function() {
      // omitted
    });

    it("should assemble URL", function() {
      // omitted
    });
  });

  describe("parseUri", function() {
    it("should parse the URI path", function() {
      // omitted
    });

    it("should parse view types", function() {
      // omitted
    });
  });

  describe("doRedirect", function() {
    // variables omitted

    beforeEach(function() {
      // omitted
    });

    afterEach(function() {
      // omitted
    });

    function assertRedirect(expectedUrl, fetchSuccess, callbackArg) {
      // omitted
    }

    it("should redirect for URLs with a blog domain using the API", function() {
      // omitted
    });

    it("should redirect for URLs with a blog domain using the blog domain as a fallback", function() {
      // omitted
    });

    it("should redirect even without the blog domain on an insights page", function() {
      // omitted
    });

    it("should redirect even without the blog domain on a stats page", function() {
      // omitted
    });
  });
});

I have one describe block for each non-util function, and each of those contains multiple test cases asserting different behaviours for the function under test. A single describe block wraps everything.

In the beforeEach of the outer describe, I set up a Sinon sandbox, which allows me to easily restore in the afterEach any stubs that I make in the tests. This helps keep the tests isolated from each other (that is, one test will not affect any other).

Notice that I defined a function called assertRedirect. Test code is still code, and just like any other code, it’s subject to refactoring. In this case, the four test cases in that describe block share a lot of the same assertion code, so I extracted the assertion code into a function. Below is the full code in that describe block.

  describe("doRedirect", function() {
    var blogDomain = "example.wordpress.com";
    var data = {URL: "url"};
    var tokens, apiFetch, oldGlobalCurrentUser;

    beforeEach(function() {
      tokens = {
        statsType: "day",
        blogDomain: blogDomain
      };
      apiFetch = sandbox.stub(statsRedirect.utils, "apiFetch");
      olGlobalCurrentUser = global.currentUser;
      global.currentUser = {
        primarySiteSlug: blogDomain
      }
    });

    afterEach(function() {
      global.currentUser = oldGlobalCurrentUser;
    });

    function assertRedirect(expectedUrl, fetchSuccess, callbackArg) {
      sandbox.assert.calledWith(apiFetch, "/sites/example.wordpress.com", sinon.match.func, sinon.match.func);

      var callback = apiFetch.firstCall.args[(fetchSuccess)? 1 : 2];
      var exampleUrl = "https://example.com/";
      var assembleUrl = sandbox.stub(statsRedirect, "assembleUrl");
      var locationReplace = sandbox.stub(statsRedirect.utils, "locationReplace");
      assembleUrl.returns(exampleUrl);

      callback(callbackArg);

      sandbox.assert.calledWith(assembleUrl, expectedUrl, tokens.statsType);
      sandbox.assert.calledWith(locationReplace, exampleUrl);
    }

    it("should redirect for URLs with a blog domain using the API", function() {
      statsRedirect.doRedirect(tokens);
      assertRedirect(data.URL, true, data);
    });

    it("should redirect for URLs with a blog domain using the blog domain as a fallback", function() {
      statsRedirect.doRedirect(tokens);
      assertRedirect("http://" + tokens.blogDomain, false);
    });

    it("should redirect even without the blog domain on an insights page", function() {
      var registerOnload = sandbox.stub(statsRedirect.utils, "registerOnload");
      tokens.blogDomain = null;
      tokens.statsType = "insights";

      statsRedirect.doRedirect(tokens);

      sandbox.assert.calledWith(registerOnload, sinon.match.func);

      registerOnload.firstCall.args[0]();

      assertRedirect(data.URL, true, data);
    });

    it("should redirect even without the blog domain on a stats page", function() {
      var registerOnload = sandbox.stub(statsRedirect.utils, "registerOnload");
      tokens.blogDomain = null;

      statsRedirect.doRedirect(tokens);

      sandbox.assert.calledWith(registerOnload, sinon.match.func);

      registerOnload.firstCall.args[0]();

      assertRedirect(data.URL, true, data);
    });
  });

Notice how extracting the common assertions into a function allowed the tests to be much shorter. There appears to be yet another opportunity for a refactor. Can you spot it?

When the tests are run, Mocha will output a report with all the passing and failing tests, like so:

If they’re all green, then our code is good to go!

The full code for this release can be found on GitHub. The main file is wpcom-stats-redirect.user.js and the test file is wpcom-stats-redirect.test.js.

Some next steps:

• Further refactor the tests.
• Possibly change the check for module and window to be a negative check for the undefined type instead of a positive check for the object type.
• Possibly eliminate the need for Sinon and Chai. Eliminating Sinon would require exploiting this binding.
• Set up a CI service like Travis CI to check each commit and pull request to make sure the tests pass before integrating them into the main branch.
• Use the same testing technique on the editor redirect script.

Patches welcome!

---

Links

• Mocha
• Sinon
• Chai
• Unit Testing and TDD in Node.js

• automated tests
• Chai
• classic stats redirect script
• Mocha
• Node.js
• Sinon
• unit tests
• user script