Test coverage and jshint

remoteStorage.js
1

i want to set up a way to see which parts of the code are not covered by any unit tests, and which parts do not comply with a basic style guide we can define.

i am experimenting with qunit+blanket in the ‘qunit’ branch, and will see if i can get the wireclient covered with that.

i searched for alternative frameworks, and i think qunit+blanket is the easiest set up to get started with.

teste is probably hard for new contributors to get familiar with, and mocha and jasmine seem to be equivalent to qunit but not as super-simple to get started with.

doing this means we will have some duplication between our existing teste suites and the new qunit tests, but i guess that’s inevitable unless we want to stay with teste for all our testing.

at this point it seems teste doesn’t support visualization of coverage, and it seems to me using a test framework that nobody else uses is an unnecessary risk.

i want to increase the number of tests anyway, so i think now would be a good time to switch from teste to qunit+blanket. counter-proposals welcome

2

ps @raucao is already looking at adding a jshint config to our codeclimate thingy

3

No, that one goes in the repo, so everybody uses the same in their editors.

4

I think duplication is a worse situation that just adding the missing tests. I’d propose to either switch the entire suite, or add the required features to Teste.

It’s also completely confusing for contributors, and if we do it in master, nobody will ever clean up the old ones or port them soon.

5

You do realize, that the title of this topic is “Test coverage and JSHint” and then you talk about switching the test suite to QUnit?

6

A couple thoughts:

  • What are the required features that teste is lacking? I have not heard of anything so far, and have not seen any feature requests or discussion of things which are lacking.

FWIW (from a discussion in irc) “sinon-like” behavior (mocks, stubs etc) exists already in teste (https://github.com/silverbucket/teste/blob/master/README.md):

var mock = new test.Stub(function(p1, p2) {
    console.log('hello world');
});

mock.called;  // false
mock.numCalled;  // 0
mock();  // hello world
mock.called;  // true
mock.numCalled;  // 1

  • As for metrics, we’ve already started using https://codeclimate.com/github/remotestorage/remotestorage.js for code complexity measurements, and overall quality control.

  • “teste is probably hard for new contributors to get familiar with”. Considering we have a wealth of existing tests in place, and the format of the tests are very similar to qunit already, I don’t really see how this statement has been qualifieid. For example, everyone is writing tests and I haven’t trained anyone as to how to use teste. It’s pretty straightforward IMO. Most issues with learning a test framework are more about learning how to test than they are about learning the test framework. If there are things which are unclear I’m happy to explain or write some more documentation, but I think that testing frameworks which need tons of documentation to learn how to use are not exactly selling points for why we should switch.

  • @michielbdejong - I’m happy to see you’re finally coming around to the opinion that tests are a valuable thing to have in a project, but getting excied about tests then deciding to re-do everything doesn’t seem like a very good use of time, especially considering we have several bugs, and some show-stopping ones (I currently can’t even use caching in my remotestorage apps) in the library that should be getting priority attention. That’s kind of the first rule about testing in general, is that we don’t make arbitrary decisions without something to back it up and confirm that we have a problem, and that our actions are going to be improving our situation.

We should be putting our efforts into expanding our test coverage rather than re-writing “just because” teste is not a popular test framework. Test frameworks are tools, they serve a purpose, and unless our tools are clearly lacking and can’t be fixed then I don’t see any reason to start from scratch. Can someone tell me some real thing we’re suffering from here?

1 Like
7

I agree with most everything, and somehow I overlooked the mocking features.

My main problem with Teste so far was the lack of contexts/groups and proper setup and teardown functions for these. Also, I think it’s missing some asserts like for errors thrown for example.

8

Cool!

  • context/groups - Point taken, I will look into adding these as this is really just an issue of adding command-line parameter support, which I haven’t got around to yet. However, since I recently added command-line parameter support to Sockethub, so am familiar with it, I think it’ll be a quick fix.

  • setup and teardown - each test has it’s own setup and takedown property which, if it’s a function, will be executed. The same exact behavior as suite-level setup and takedown properties.

For example:

{
  desc: "example of a test with it's own setup and takedown functions",
  setup: function (env, test) {
    env.fooBar = 'this will be available during the test';
    test.done();
  },
  run: function (env, test) {
    test.assert(env.fooBar, 'this will be available during the test');
  },
  takedown: function (env, test) {
    // cleanup code
    delete env.fooBar;
    test.assertType(env.fooBar, 'undefined');
  }
}

Good point, I think this is due to the use of the promising library not able to catch certain cases where errors are thrown. I plan to switch to the Q promise library and this should hopefully improve the behavior in this regard.

9

Ah, didn’t know that. (So, maybe we can improve the Teste docs after all).

If/when you add contexts, then they should support those as well.

10

By the way, just as a hint, you can wrap code examples in backtick blocks with a language identfier, like so e.g.:

```js

[code]
```

That will add code highlighting and you don’t have to indent.

11

ok, let’s forget about visualizing test coverage then for now, then we can stick with teste.

only problem then - how can we test the widget? do we currently have any tests for the widget at all?

12

Widget and View don’t have any tests that I know of, but we did not have any issues about the widget for quite a while(except this #471), so I have the impression that it works fine right now.

Other parts also don’t have testing and I because I don’t realy know how to test the view properly I would focus on other parts of test coverage.

13

ok, that sounds reasonable, let’s do that then, and forget about widget testing for now