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 logo

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 logo

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 logo

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!


27 comments
  1. Thank you, Penguin for all your time and energy put into this most important script for all of us bloggers! Cheers!

    • Hmm, I’ve always thought that this was of lesser importance than the editor script; that’s why I experimented on this first. I barely look at my stats nowadays. But hey, if you find use for it, then you’re most welcome.

      • My fault for not reading this properly. I thought it was for the class editor script. I use the very old Stats page and very rarely use the new Blue one.

      • Yes, I use your script for the Stats Sparkline, thank you for that one, too. I just use the wp admin Stats from a bookmarked tab. Maybe I should try this script, too.

  2. Hi Penguin, this is not strictly connected to the stats page. But I just got a header across my admin page that the Stylish add-on will not work with the latest version of Firefox. But I clicked on the Stylish icon anyway with my stats page open and lo and behold it still gave me the “wide stats” view. So I’m not sure what they’re talking about. I was directed
    to Stylus here instead.

    I had a look, and there is so much coding and geek talk that I chickened out. Perhaps you can make head or tails of this?

    • Gladly. Mozilla, the company who makes Firefox, is moving towards a new format for add-ons called “WebExtensions”, based on the way Chrome does it. Stylish is not written as a WebExtension and instead uses the older XUL platform. As of Firefox 57, which should be out in the next week or so, Firefox will completely drop support for XUL add-ons, and because Stylish is a XUL add-on, it won’t work in Firefox 57 (the “latest version”). You’ve been directed to Stylus because it’s a WebExtension.

      I personally don’t agree with this change and moved away from Firefox to Pale Moon a long time ago, but I’m not sure it would be wise to suggest the same for less technical people.

      There’s a way for you to migrate your user styles from Stylish to Stylus, but you have to do it before you upgrade to Firefox 57, so I would suggest doing it soon. You can follow the instructions in the FAQ. Let me know if you’re stuck.

      • Many thanks for your reply. It looks a bit daunting, but on the other hand I’m not that bothered about the wide stats page. It is a very nice option but if it is unavailable I will manage.

        However what does bother me is the greasemonkey add-on for the classic editor which you wrote. Will that still work in Firefox 57? That is more important to me than anything else. I cannot abide the “new” wordpress editor even though it has been improved somewhat.

      • Greasemonkey v4 is being worked on to bring compatibility for the new version. I encourage you to read the “Greasemonkey 4 For Users” as well to see what your alternatives are in case Greasemonkey 4 doesn’t work out for you when it comes out. The posts says to check with the original author of your scripts, and I can say that none of my scripts need any changes to make them compatible.

        I guess this means I’ll have to grab a copy of Firefox 57 for testing when the time comes.

      • Ok, I have managed to download the xpi and have crated a json on my desktop. But now I can’t add the Stylus add-on to Firefox! Is that because I’m still on FF 56?

        I do apologize for my dumb questions and taking over your comment section.

      • Stylus requires at least version 52, so you should be fine. What’s the error you’re getting?

        Don’t worry about taking over the comment section. I’m sure someone else will ask about this in the future, and when that happens, I can point them to this thread.

      • It says “the add on could not be downloaded because of a connection failure”.

        Obviously my own internet is working. Maybe their site crashed?

      • Perhaps. Try clearing your cookies and trying again in a few hours.

        Alternatively, install it through the add-ons page (“about:addons” in your URL bar, or press Ctrl+Shift+A/Cmd+Shift+A). It will probably work there.

      • OK, I’ll give it a try. Many thanks!

      • Thank you very much for your reassuring reply re Greasemonkey. I will have a look at the user guide but you have to understand I have zero background in any of this! I’m on a steep learning curve, mainly due to your good efforts. I can’t thank you enough.

      • “Greasemonkey v4 is being worked on to bring compatibility for the new version” – I have version 3.17 installed at the moment. Do I have to force an update or will it do it automatically when FF 57 comes out?

      • I don’t see why it shouldn’t update itself when they release v4. If it doesn’t by the time you upgrade to v57, you can always force an update or reinstall the add-on.

      • OK, thank you. Worst case, I will bombard you with more questions! :-D

      • Hi Penguin, I cleared all my cookies and history, went to Mozilla’s addon page, and tried ot install Sylus. But I still get a “connection failure” message. This is Stylus Beta. Maybe they have a Sylus Alpha that would work? (only half kidding).

      • Did you try the other method I mentioned? I think that’s more likely to work. If that fails, you can try downloading the XPI with another browser and installing that instead.

        Funny enough, developers do use alpha versions, but they’re usually even less stable than beta versions (alpha turns to beta after it becomes more stable). I haven’t seen anyone use gamma, though; after beta is usually “rc” (“release candidate”). Coding Horror has a post about it if you’re interested.

      • The reviews of Stylus are mixed at best. Maybe I’ll just leave it for now.

      • Unfortunately, it’s the only real alternative available for v57, although reading the negative reviews, it makes me wonder if they read the migration guide. I’ve used Stylus myself and so far it’s simply been “set it and forget it”.

        reStyle is also available, but it uses a different mechanism (requiring a browser restart whenever you change styles) and will probably be more confusing in the long-run.

      • Yay! I did it! It suddenly dawned on me that I had to have the stats page open because that is the only site which uses Stylish, ergo the only page that would use Stylus too. And then it worked. I have imported the json file successfully (according to Stylus anyway!) so I hope it will all work properly on the next FF upgrade.

        Thank you so much again for your help. :-)

  3. thank you so much. apart from absolutely hating every “update” that makes wordpress less and less useful, which im being constantly dragged through like we are red vs blue and only blog here to provide training for wp dev freelancers, i tried to load the “new, improved” editor today and only part of it loaded!

    wait, hold on– so i turned OFF greasemonkey AND stylish, deleted userContent.css (using pale moon, not sure that file even does anything, i added it yesterday before installing stylish) and reloaded the editor– STILL nothing! and it worked fine last night– no telling if its them or me.

    so i reinstalled your redirect userscript– which im glad exists though i havent used it in a while– and hey! now i can edit (and write) blogs again. thank you for your wonderful script. not only a good idea in principle, but wonderfully useful in practice! youre doing something excellent. cheers.

    • You’re very welcome. Glad to hear everything works again!

      By the way, userContent.css does work in Pale Moon, but Stylish (or the various equivalents) is a much better solution anyway because userContent.css requires a restart to apply the changes.

      • brilliant! thanks.

`$name' says...

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.

%d bloggers like this: