Permalink
Browse files

Don't kill process on errors that we can pin to a test

This goes back on: e509041 where I made
runSuite kill the process when it encountered any errors.  Now it only
does that when it encounters errors that it cannot figure out which
tests caused them.  That will only happen in parallel mode.

Why have worse results all the time when it is really only running in
parallel that causes them?
  • Loading branch information...
1 parent 57f9f7a commit 6ea37a537c542ccce6b2e6844d5632f41209c9a5 Benjamin Thomas committed Oct 30, 2010
View
@@ -416,7 +416,7 @@ <h2>Events</h2>
Called when a suite finishes. See <a href="#suite-result">Suite result</a>
below.
<code>status</code> is one of the following:<br> <b>complete</b>, <b>error</b>,
- <b>loadError</b> or <b>exit</b>.
+ <b>exit</b> or <b>loadError</b>.
</dd>
</dl>
@@ -498,7 +498,8 @@ <h2 id="test-result">Test Result</h2>
<p>the test failed in some way</p>
<div class="highlight"><pre>
{ name: test name
-, failure: the assertion failure
+, failureType: 'assertion' or 'error' depending on how this failed
+, failure: the error that caused this to fail
}</pre></div>
</dd>
</dl>
View
@@ -130,11 +130,39 @@ <h1 id="tests">Tests</h1>
<span class="nx">test</span><span class="p">.</span><span class="nx">finish</span><span class="p">();</span>
<span class="p">}</span></pre></div>
- <p>
- <b>Note:</b> it is important that no code from this test is ran after
+ <h2>Tip:</h2>
+ <p> It is important that no code from this test is ran after
this function is called, otherwise, <b>node-async-testing</b> will think
that code corresponds to a different test. Be careful!
</p>
+
+ <p>
+ If <code>test.finish()</code> is called more than once, or an assertion is made after <code>finish()</code>
+ has already been called, an error will be thrown. These features help catch test
+ errors. Use these to your advantage. For example, at the beginning of an asynchronous
+ callback make an assertion! If the test has already finished (the callback is being called
+ when it isn't supposed to be) then the assertion will catch that the test has already finished
+ and prevent the callback from running any code that can interfere with other tests. Example:
+ </p>
+
+ <div class="highlight"><pre>
+<span class="nx">module</span><span class="p">.</span><span class="nx">exports</span> <span class="o">=</span>
+ <span class="p">{</span> <span class="s1">&#39;okay&#39;</span><span class="o">:</span> <span class="kd">function</span><span class="p">(</span><span class="nx">test</span><span class="p">)</span> <span class="p">{</span>
+ <span class="nx">setTimeout</span><span class="p">(</span><span class="kd">function</span><span class="p">()</span> <span class="p">{</span>
+ <span class="cm">/* run code */</span>
+ <span class="nx">test</span><span class="p">.</span><span class="nx">finish</span><span class="p">();</span>
+ <span class="p">}</span>
+ <span class="p">}</span>
+ <span class="p">,</span> <span class="s1">&#39;GOOD PRACTICE!&#39;</span><span class="o">:</span> <span class="kd">function</span><span class="p">(</span><span class="nx">test</span><span class="p">)</span> <span class="p">{</span>
+ <span class="nx">setTimeout</span><span class="p">(</span><span class="kd">function</span><span class="p">()</span> <span class="p">{</span>
+ <span class="nx">test</span><span class="p">.</span><span class="nx">ok</span><span class="p">(</span><span class="kc">true</span><span class="p">);</span> <span class="c1">// make sure test isn&#39;t already finished</span>
+
+ <span class="cm">/* run code */</span>
+
+ <span class="nx">test</span><span class="p">.</span><span class="nx">finish</span><span class="p">();</span>
+ <span class="p">}</span>
+ <span class="p">}</span>
+ <span class="p">};</span></pre></div>
</dd>
<dt id="test.numAssertions"><code>test.numAssertions</code></dt>
@@ -153,6 +181,13 @@ <h1 id="tests">Tests</h1>
<span class="nx">test</span><span class="p">.</span><span class="nx">ok</span><span class="p">(</span><span class="kc">true</span><span class="p">);</span>
<span class="nx">test</span><span class="p">.</span><span class="nx">finish</span><span class="p">();</span>
<span class="p">}</span></pre></div>
+ <p>
+ If you are testing asynchronous code, I <i>highly</i> recommend using <code>test.numAssertions</code>.
+ <b>node-async-testing</b> depends on <code>test.finish()</code> to tell it when a test is finished,
+ so if code is run after <code>finish()</code> was called you will get innacurate test results. Use
+ <code>numAssertions</code> to be sure all your callbacks are called <i>and</i> that they aren't called
+ too many times.
+ </p>
</dd>
<dt id="test.uncaughtExceptionHandler"><code>test.uncaughtExceptionHandler</code></dt>
View
@@ -174,7 +174,8 @@ exports.run = function(list, options, callback) {
}
if (result.failure) {
- console.log(red('' + result.name));
+ var color = result.failureType == 'assertion' ? red : yellow;
+ console.log(color('' + result.name));
}
else if (options.printSuccesses) {
console.log('' + result.name);
@@ -231,9 +232,14 @@ exports.run = function(list, options, callback) {
for(var i = 0; i < tests.length; i++) {
var r = tests[i];
if (r.failure) {
- console.log(' Failure: '+red(r.name));
var s = r.failure.stack.split("\n");
- console.log(' '+ s[0].substr(16));
+
+ console.log(r.failureType == 'assertion' ? ' Failure: '+red(r.name) : ' Error: '+yellow(r.name));
+
+ if (r.failure.message) {
+ console.log(' '+ r.failure.message);
+ }
+
if (options.verbosity == 1) {
if (s.length > 1) {
console.log(s[1].replace(process.cwd(), '.'));
@@ -295,7 +301,7 @@ exports.run = function(list, options, callback) {
console.log('');
if (tests.length > 1) {
- console.log(' One of the following tests threw an error: ');
+ console.log(' Stopped Running suite! Cannot determine which test threw error: ');
console.log(' ' + names);
}
else {
View
@@ -2,8 +2,17 @@ var assert = require('assert')
, path = require('path')
, fs = require('fs')
, spawn = require('child_process').spawn
+ , util = require('util')
;
+var TestAlreadyFinishedError = function(message) {
+ this.name = "TestAlreadyFinishedError";
+ this.message = message;
+ Error.captureStackTrace(this);
+};
+util.inherits(assert.AssertionError, Error);
+TestAlreadyFinishedError.__proto__ = Error.prototype;
+
/* Runs an object of tests. Each property in the object should be a
* test. A test is just a method.
*
@@ -108,7 +117,7 @@ exports.runSuite = function(obj, options) {
test.obj[fn] = function() {
// if the test doesn't have a func, it was already finished
if (!test.func) {
- testAlreadyFished(test, 'Encountered ' + fn + ' assertion');
+ testAlreadyFinished(test, 'Encountered ' + fn + ' assertion');
}
try {
assertionFunctions[fn].apply(null, arguments);
@@ -126,7 +135,7 @@ exports.runSuite = function(obj, options) {
}
function testAlreadyFinished(test, msg) {
- errorHandler(new Error(test.name + ' already finished!' + (msg ? ' ' + msg : '')), test);
+ errorHandler(new TestAlreadyFinishedError(test.name + ' already finished!' + (msg ? ' ' + msg : '')), test);
}
// this is called after each step in a test (each function in the array).
@@ -181,10 +190,9 @@ exports.runSuite = function(obj, options) {
}
function testFinished(test, problem) {
- if (test.failure) {
- delete test.numAssertions;
- }
- else if (test.obj.numAssertions && test.obj.numAssertions != test.numAssertions) {
+ if (!test.failure
+ && test.obj.numAssertions
+ && test.obj.numAssertions != test.numAssertions) {
// if they specified the number of assertions, make sure they match up
test.failure = new assert.AssertionError(
{ message: 'Wrong number of assertions: ' + test.numAssertions +
@@ -194,6 +202,11 @@ exports.runSuite = function(obj, options) {
});
}
+ if (test.failure) {
+ test.failureType = test.failure instanceof assert.AssertionError ? 'assertion' : 'error';
+ delete test.numAssertions;
+ }
+
// remove it from the list of tests that have been started
suite.started.splice(suite.started.indexOf(test), 1);
@@ -252,19 +265,28 @@ exports.runSuite = function(obj, options) {
}
}
- process.removeListener('uncaughtException', errorHandler);
- process.removeListener('exit', exitHandler);
-
- if (options.onSuiteDone) {
- var tests = test ? [test] : suite.started;
- if (tests.length < 1) {
- tests = suite.results;
- }
- options.onSuiteDone('error', { error: err, tests: tests.map(function(t) { return t.name; })});
- process.exit(1);
+ if (!(err instanceof TestAlreadyFinishedError) && (test || suite.started.length == 1)) {
+ // if we can narrow down what caused the error then report it
+ test = test || suite.started[0];
+ testProgressed(test, err);
}
else {
- throw err;
+ // otherwise report that we can't narrow it down and exit
+ process.removeListener('uncaughtException', errorHandler);
+ process.removeListener('exit', exitHandler);
+
+ if (options.onSuiteDone) {
+ var tests = test ? [test] : suite.started;
+ if (tests.length < 1) {
+ tests = suite.results;
+ }
+ options.onSuiteDone('error', { error: err, tests: tests.map(function(t) { return t.name; })});
+ process.exit(1);
+ }
+ else {
+ // TODO test this
+ throw err;
+ }
}
}
@@ -96,12 +96,17 @@ io.setPath('/client/');
// normal error
var test = suite.tests[obj.result.name];
var el = test.el;
- var status = obj.result.failure ? 'failure' : 'success';
- removeClass(el, 'running');
- el.className += ' ' + status;
+ removeClass(el, 'running');
if (obj.result.failure) {
- suite.numFailures++;
+ if (obj.result.failureType == 'assertion') {
+ suite.numFailures++;
+ el.className += ' failure';
+ }
+ else {
+ suite.numErrors++;
+ el.className += ' error';
+ }
var code = document.createElement('CODE');
code.className = 'result';
@@ -113,13 +118,19 @@ io.setPath('/client/');
summarySpan.innerHTML = (obj.result.failure).message;
test.nameEl.appendChild(summarySpan);
}
+ else {
+ el.className += ' success';
+ }
}
, suiteDone: function(obj) {
suite = suites[obj.suite];
var el = suite.el;
- if (suite.numFailures > 0) {
+ if (suite.numErrors > 0) {
+ var status = 'error';
+ }
+ else if (suite.numFailures > 0) {
var status = 'failure';
}
else {
View
@@ -78,7 +78,8 @@ exports.wrap = function(obj) {
if (obj.suiteSetup || obj.suiteTeardown) {
n.push(function(t, f) {
switch(state) {
- case SETUP: // suiteSetup is but it just finished... oops!
+ case SETUP:
+ // we still think suiteSetup is still running but some test just finished... oops!
state = SETUPFAILED;
for (var i = 0; i < pendingTests.length; i++) {
fail(pendingTests[i][0]);
View
@@ -5,7 +5,9 @@
// As such, you have to manually look at the quite cryptic output to make sure
// it is doing what you want.
-
+if (module == require.main) {
+ return require('../../lib/async_testing').run(process.ARGV, function() {/* do nothing */});
+}
var order = ''
// Which functions should be run async or sync. Each slot is a different flow
@@ -15,25 +17,43 @@ var order = ''
// specify which tests to create. P means that function should pass,
// F means it should fail
, tests =
- { A: 'PPP'
- , B: 'FPP'
- , C: 'PFP'
- , D: 'PPF'
- , E: 'FPF'
- , F: 'PFF'
- , G: 'PPPPP'
- , H: 'FPPPP'
- , I: 'PFPPP'
- , J: 'PPFPP'
- , K: 'PPPFP'
- , L: 'PPPPF'
- , M: 'FPPPF'
- , N: 'PFPFP'
- , O: 'PFPPF'
- , P: 'PPFFP'
- , Q: 'PPFPF'
- , R: 'PPPFF'
- };
+ { AA: 'PPP'
+ , AB: 'FPP'
+ , AC: 'PFP'
+ , AD: 'PPF'
+ , AE: 'FPF'
+ , AF: 'PFF'
+ , AG: 'PPPPP'
+ , AH: 'FPPPP'
+ , AI: 'PFPPP'
+ , AJ: 'PPFPP'
+ , AK: 'PPPFP'
+ , AL: 'PPPPF'
+ , AM: 'FPPPF'
+ , AN: 'PFPFP'
+ , AO: 'PFPPF'
+ , AP: 'PPFFP'
+ , AQ: 'PPFPF'
+ , AR: 'PPPFF'
+ , AS: 'EPP'
+ , AT: 'PEP'
+ , AU: 'PPE'
+ , AV: 'EPPPP'
+ , AW: 'PEPPP'
+ , AX: 'PPEPP'
+ , AY: 'PPPEP'
+ , AZ: 'PPPPE'
+ , BA: 'EEP'
+ , BB: 'EPE'
+ , BC: 'PEE'
+ , BD: 'EFP'
+ , BE: 'EPF'
+ , BF: 'PEF'
+ , BG: 'FEP'
+ , BH: 'FPE'
+ , BI: 'PFE'
+ }
+ ;
for (var key in tests) {
combinations(tests[key]).forEach(function(t) {
@@ -60,6 +80,22 @@ for (var key in tests) {
}, 10);
});
}
+ else if (f == 'E') {
+ name += 'error ';
+ test.push(function(t) {
+ order += (index == 1 ? '\n' : '')+k+index+'!';
+ throw new Error('error in '+index);
+ });
+ }
+ else if (f == 'e') {
+ name += 'aerror ';
+ test.push(function(t) {
+ setTimeout(function() {
+ order += (index == 1 ? '\n' : '')+k.toLowerCase()+index+'!';
+ throw new Error('error in '+index);
+ }, 10);
+ });
+ }
else if (f == 'F') {
name += 'fail ';
test.push(function(t) {
@@ -91,7 +127,7 @@ function combinations(list, spot) {
var r = [];
for (var i = 0; i < right.length; i++) {
- if (runSyncAsync[spot] == 'S') {
+ if (list[0] == 'E' || runSyncAsync[spot] == 'S') {
r.push([list[0]].concat(right[i]));
}
else {
@@ -104,7 +140,7 @@ function combinations(list, spot) {
else {
var r = [];
- if (runSyncAsync[spot] == 'S') {
+ if (list[0] == 'E' || runSyncAsync[spot] == 'S') {
r.push(list[0]);
}
else {
@@ -114,10 +150,7 @@ function combinations(list, spot) {
}
}
-process.on('exit', function() {
- console.log(order);
-});
-if (module == require.main) {
- require('../../lib/async_testing').run(__filename, process.ARGV);
-}
+setTimeout(function() {
+ console.log(order);
+}, 500);
Oops, something went wrong.

0 comments on commit 6ea37a5

Please sign in to comment.