#151: Add unit tests for JavaScript.

[Osmose]

Extracts the API from browserid.js and moves it to a new file, api.js, that authors can include on their own if they don't want the automatic behavior of browserid.js. This also allows us to unit-test the API while ignoring the onload stuff from browserid.js.

There aren't tests yet for browserid.js as that would better be handled by end-to-end tests using something like Selenium.

[Osmose]

@alexgibson @willkg r? One person for a frontend-centric view, and one for a django-browserid-centric view. :D

[willkg]

I can look at the backend stuff (and possibly the frontend stuff) tomorrow (Friday).

[willkg]

There isn't much backend stuff. (That's what I thought you meant by "django-browserid-centric view".) I ran the tox tests and the karma tests, so the documentation changes are good.

I went through the api.js changes and those look ok as far as I can tell.

I've never used sinon or karma, so I don't know enough to review that code effectively. But it is easy to read which I like. Reminds me of Dr. Seuss in some places.

So... Things look ok to me as far as I can tell, but I'm not sure how useful that is and it's probably prudent to have someone else skim through it, too.

[Osmose]

Oh yeah, I forgot to add config stuff to .travis.yml, will add that when I get a chance.

[alexgibson]

Looking good :+1:

This is maybe a personal pref thing, but one small suggestion I would make would be be to consider using a more descriptive approach to writing the tests. So instead of using a comment in the JS file to describe the desired behavior for each test, you could word them so like so:

describe('login', function(){
    it('should fetch an assertion and pass it to verifyAssertion', function(){
      // assert
    });
});

You only have a small number of tests here, so perhaps this is overkill. But I find this approach is useful especially when tests fail, as I find it quicker/easier to identify what's not working.

On methods where you have more than one test, it can also read a little nicer (imho)

describe('registerWatchHandlers', function(){
    it('should pass the userEmail from the info response to navigator.id.watch', function(){
      // assert
    });
    it('should log the user out if login fails, ', function(){
      // assert
    });
});

I haven't used Mocha, but it looks like it has a couple of different interface options for writing tests:

http://visionmedia.github.io/mocha/#interfaces

Sticking with the tdd interface you currently have, you could also write the tests like so:

suite(registerWatchHandlers', function(){
    test('should pass the userEmail from the info response to navigator.id.watch', function(){
      // assert
    });
    test('should log the user out if login fails', function(){
      // assert
    });
});

Just some food for thought, otherwise nice work!

[Osmose]

On methods where you have more than one test, it can also read a little nicer (imho)

Agreed! Unfortunately, we only have two cases of that, and in the case of a single test, it ends up adding an extra level of indentation and is a bit verbose.

I will definitely take the advice about moving the descriptions from comments to test names though. :D