iamchrismiller / grunt-casper

Run CasperJS Scripts/Functional Tests
Other
69 stars 38 forks source link

Grunt task completes before all casperjs spawn complete #51

Closed sebastienba closed 10 years ago

sebastienba commented 10 years ago

It seems that when running multiple tests suites for instance { files : { 'xunit/casper-results.xml' : ['product/test.js'] ,'xunit/casper-results2.xml' : ['product/test2.js']

      }
  }

The grunt task completes when the first casperjs test suite complete, the other spawn still proceed but competes with the following grunt tasks.

You basically invoked taskComplete for each spawn instead of waiting for all spawns to complete and call it once.

iamchrismiller commented 10 years ago

Thank you for letting me know about this. I have fixed it in v0.4.0 and added a test for this case.

sebastienba commented 10 years ago

I did a quick code review, I'm not sure if the fix will solve the issue i faced, basically, when one or more test fail, there are pending casper instances that will still write in stdout, running this in jenkins trigger all sort of errors that jenkins prevent it's own way. The issue is in the implementation of grunt.util.async.forEachLimit itself. It surely prevent the callback to be called more than once, which is good but it doesn't delay it's invocation. I had a temporary fix using queue instead of forEachLimit: //Run Tests In Parallel if (file.src) { var worker = function(data, callback) { casperLib.execute(data.srcFile, data.dest, options, args, callback);
}; var queue = grunt.util.async.queue(worker, concurrency); var oneError = null; // callback for each testsuite, which will only conclude the grunt task once spawns stopped. var callBack = function(err) { if (err) { oneError = err; grunt.log.write('error:', err); } if (queue.running() == 0 && queue.length() == 0) { taskComplete(oneError);
} } file.src.forEach(function(srcFile) { queue.push({ srcFile : srcFile , dest: file.dest !== 'src' ? file.dest : null }, callBack); }); queue.process(); }

iamchrismiller commented 10 years ago

Looks good.. I will look into moving this over to Async.queue early this week sometime. It looks like a cleaner implementation.