Add "teardown" option (counterpart to "bootstrap")

[pigulla]

I'm using Wallaby to run tests in parallel, so in my integration tests I need to use a different Mongo database per worker. This is achieved easily enough by appending process.pid to the default database name in Wallaby's bootstrap function.

However, it is not clear to me how to drop that database when the test run is complete. If I don't, my Mongo will end up being cluttered by countless "dead" databases.

Ideally, one would use the test framework's global after hook. But unfortunately, popular frameworks like Mocha do not support this (there are proposed workarounds, but in this case they are no good).

I could do the database setup and teardown each testcase's before and after hooks, but not only is this unnecessary, it will also have a notable performance impact and potentially cause other issues like having to manually recreate Mongo indices.

Also, since Mocha doesn't support parallel execution natively, one could argue that handling this sort of setup and teardown is best located in Wallaby.

It seems to me that adding a (asynchronous) teardown option to Wallaby's config would be fairly simple to implement and solve this issue very elegantly.

Alternatively, a "less perfect" but probably easier to implement solution could be to expose some sort of "worker id" (e.g., its number between 1 and workers.initial). If that number was available it could be used instead of the process id to create a database specific to that process and those databases could be reused, essentially rendering the cleanup step above unnecessary.

Thoughts?

[ArtemGovorov]

While I like the idea and think it's quite useful feature to have, in your specific case can't you just do this?

bootstrap: function (wallaby) {
      var mocha = wallaby.testFramework;
      // DB setup code
      mocha.suite.afterAll(function () {
        console.log('afterAll', process.pid);
        // DB clean up code
      });
    }

[pigulla]

Interestingly, mocha.suite seems to have an afterAll method but no after (which is what I would want to use) :-(

[ArtemGovorov]

Doesn't the proposed afterAll callback in the code that I have suggested do exactly what you've suggested for the teardown? It looks like it gets executed once per run (no matter how many test files are in the run).

[pigulla]

Indeed it does, my apologies. Is suppose having afterAll documented on Mocha's website would have been helpful.

Thanks for the help, feel free to close this.

[ArtemGovorov]

Awesome, thanks.

I'll leave it opened if you don't mind because I think it's a good idea to have the teardown function anyway. So if there'll be some other use cases from you or others I'll jump into it and quickly add the feature.

[bestan]

I did notice that if I do something like this

bootstrap: function(wallaby) {
    console.log("beforeAll", process.pid);
    var mocha = wallaby.testFramework;
    mocha.suite.afterAll(function () {
        console.log('afterAll', process.pid);
    });
},

and start breaking things quickly (i.e. calling invalid functions, then quickly correcting it etc.), then I'm getting more beforeAlls than afterAlls (i.e. 13 beforeAlls, and 7 afterAlls). I'm guessing that mocha is not dealing with all uncaught exceptions, crashing and not running afterAll properly.

In this case, having independent teardown is quite useful - it simplifies dealing with bugs and uncertainties of individual test frameworks.

P.S. I'm running tests with both Mongo and http server. I can drop the database before running my tests and not rely on afterAll, but killing http server without references seems to be complicated (if possible).

@ArtemGovorov @pigulla thoughts?

[ArtemGovorov]

@bestan @pigulla Indeed afterAll callback is not always called. Nice catch @bestan

So I went ahead and have added the teardown function to wallaby.js. For now it's a sync function for simplicity and consistency of the implementation.

Have also added workerId (a number between 0 and MAX(workers.initial, workers.regular) - 1) as a property of the bootstrap function parameter.

It is implemented in core v1.0.105.

@pigulla Even though the teardown function exists now, I'd actually recommend using your idea of creating a database per worker id and re-using it. Not only it's going to be faster, but also no matter how reliable the teardown function is, there are still cases when it will not be triggered.

For example, if you introduce an infinite loop in your code, and keep changing it, wallaby.js will have to force kill the node worker because it will not be responsive and can not be gracefully interrupted due to the single-threaded nature of the JavaScript event loop. In this case there's just no way to run the teardown code in the context of that non-responsive occupied worker.

The issue is not specific to wallaby, any test runner will not be able to run any teardown code in this case. It's just because wallaby runs your tests more frequently, there's a greater chance to hit the issue of the teardown function not being called.

Of course there's an option to introduce another teardown function that may be executed within the wallaby main process as opposed to the worker. The drawback is that it'll obviously not have any worker context.

@bestan

but killing http server without references seems to be complicated (if possible)

One option for you would be to create http servers without passing a specific port in your tests, in this case node will assign some random available port, so you will not have to worry about releasing it.

If you're spinning up your http server in the bootstrap function, then you may obviously just dispose it there as well (or save to something like global._serverToKill and kill it in the teardown function). But I am assuming you (or some node modules) are spinning it up somewhere in your tests. In this case, I'd monkey-patch the http.createServer function to track all references and kill them later before the next start, for example:

    bootstrap: function (wallaby) {
      (global._httpServers || []).forEach(function (server) {
        try {
           // kill server here, 
           // may need to destroy sockets if immediate kill is required: http://stackoverflow.com/questions/14626636/how-do-i-shutdown-a-node-js-https-server-immediately
        }
        catch(e) {}
      });
      global._httpServers.length = 0;

      var http = require('http');
      // to avoid patching it more than once
      if (!http._originalCreateServer) {
        http._originalCreateServer = http.createServer;
        http.createServer = function() {
          var newServer = http._originalCreateServer.apply(this, arguments);
          global._httpServers = global._httpServers || [];
          global._httpServers.push(newServer);
          return newServer;
        }
      }
    }

[bestan]

Got my use cases working. Thanks @ArtemGovorov !

P.S. you've pretty much read my mind when added workerId 👍

[pigulla]

Sweet, thanks!