feat(tests): unit all the tests

[kasperlewau]

• Split controller to separate file.
  • Easier to reason about in tests.
  • Easier on the end user should he/she want to modify/extend said controller.
• Add gulp karma #single-run
• Add gulp watch:karma #no-single-run

---

Some issues/possible issues I found (read: created) while having some fun with tests;

implementation

I suppose the below points (barring the first) could all be design choices for all I know. Though the JSDOC would suggest otherwise.

• module definition should probably happen in its own file now that there are multiple ones (opinion).
• choice is not ensured in this.highligt.
• choice is not ensured to follow the correct structure in this.label.
• choice is not ensured in this.encode.
• choice is not ensured to follow the correct structure in this.encode.
• match is not ensured to be a regex.exec() in this.search.

specs

• ctrlInstance is a sh_tty name. *why did I go there._
  • $mention?
  • Subject?
  • ctrl?
• Lines 294-317 of mentionController.es6 are completely untested.
• keyboard events s-ck in phantomjs (switch to browser launchers?)
• $element.scrollHeight testing is sort of wonky.
• A lot of the tests could probably be simplified with the help of some more ES6.

build

• Multiple use stricts s-ck, "Gee, thanks kasper..."

---

Figured I'd post this before spending too much time on taking care of the aforementioned points. Not even certain you would want to roll Mocha > Jasmine, so this may all have been a couple of hours of all-fun no-work. I'm cool with that!

Ain't nobody got time for that.

but but, I like testing... 😯

[kasperlewau]

Doing a quick sanity check before going to bed here. Some of the tests are malicious in that they create expectations on the internal dependency between the exposed API methods.

Which is not good as it:

• Is brittle.
• Makes any sort of refactor harder.

input and ouput is the name of the game. Anything inbetween is silly. I'll give the suite a touch up in the morning.

[ProLoser]

@kasperlewau is this a one-time thing or would you be interested in becoming the repo maintainer?

[kasperlewau]

@ProLoser I wouldn't say this is a one-time thing, as I have slowly started dabbling with #2 as well as handling the aforementioned issues in this PR. Got some more ideas floating around in my head as well that I would like to see handled at some point, they sort of tie into the idea of being able to customise the API.

I think this is a good starting point as I'm not (yet) fully up to speed with all the bits and bobs of the codebase, so having a reliable test suite will only ever assist me (and others) in future development.

With all of that said, I'm not necessarily interested in becoming the maintainer of the repo at this time, but I'm certainly not ruling it out for the future. I would have to have a little think about it.

I hope that's not the answer you were not looking for.

[ProLoser]

@kasperlewau i would test what the methods currently do to ensure they do their job properly. If people override the methods then those tests are isolated, so I don't really understand what the issue is too much.

[kasperlewau]

@ProLoser I am all for testing what the methods are currently doing, what I am not for is what I previously did in testing the corrolation between the exposed API methods.

Take the following (somewhat silly example) for instance;

function a (wat) {
  b(wat);
}

function b (watwat) {
  watwat = `${watwat}something`
} 

var bar = 'bar';
a(bar);

Instead of testing that function a called function b, the expectation should be that bar is now barsomething, if that makes any sense. If you take a look at my latest commit, a lot of spies were used to test the execution of another function, as opposed to testing the output.

---

The issue is not so much in regards to when and if a consumer modifies one of the exposed API methods, so much as it being a bad practice (from where I'm coming from) when one comes around to refactor any of these exposed methods.

So with that, I could put some more effort into it and write expectations on the result of calling another API method internally, as opposed to the actual call. That's just something not worth testing, and I'm sort of poking my head as to why I did that in the first place. Perhaps I got carried away.

I'm sorry, I'm rambling. I'm going to grab my friday beer and sit down with the specs in a wee bit. 🍻