Overdrivr / micro-ci

Continuous Integration for embedded platforms
0 stars 0 forks source link

Improve isolation between test: #64

Closed DanFaudemer closed 8 years ago

DanFaudemer commented 8 years ago

Hello Remi, Quck summary, I moved all the global variable in a describe function so they are not interacting between each test file. I also create an after function on each test using nock to be sure that we have no pending mocks wich can influence other test.

Commit

Overdrivr commented 8 years ago

Excellent job Dan !

A few questions:

DanFaudemer commented 8 years ago

Hello Rémi,

For your first point: It is usefull to have one top level describe when you include all your package. I had an issue in the test-setup package and as it was not in a describe fonction mocha was just crashing without telling me in which test it was. Are my explaination clear ?

For the second point:

DanFaudemer commented 8 years ago

Hello,

As you suggested I update the code for removing the cache. I go with clear-require which is doing what I was doing before. I tried decache but it seems it is not working and is very long to execute.

Overdrivr commented 8 years ago

My suggestion for clearRequire was more in the lines of

var foo   = require('foo'),
var clear = require('clear-require'),
            clear('../../server/server'),
var app   = require('../../server/server);

This way, it is much quicker to write and makes for less lines of code.

But this idea of clearing the cache for ../../server/server is a very good one, I realize now that for testing the best thing to do would be that each test suite should do it and instanciate its own set of models inside the database. The test-setup.js file should then be removed.

Initially I was thinking it is a good thing to test the server without clearing the cache because it is in more real-like conditions, but it's way better to isolate tests.

I have opened #66 in that regard

Overdrivr commented 8 years ago

The call to clearRequire is not working as expected

Console output during testing shows at the beginning that the server is instantiated 4 times, each one corresponding to a call to clear. It is then followed by all the tests, which means the clear('../../server/server') is not performed before each test but once at the beginning.

The proposed fix is to call clear in a before block in each test file, for each test suite that needs it.

Overdrivr commented 8 years ago

I have made the changes in the branch isolate_test, which is breaking the test since now some data is missing from some tests. I will make the appropriated changes then a new PR