Skip to content

Loading…

Handle errors correctly based on suite.options.error and the number of parameters expected by the vow #263

Merged
merged 24 commits into from

2 participants

@adamstallard

Currently, if an error occurs in a topic and a subsequent vow expects 2 or more arguments, the first (error) argument will be set to null and the second will contain the error.

Currently a reference error in a topic will be thrown.

This the expected (fixed) behavior:

Don't throw errors; catch them and handle them as follows: When suite.options.error is set to false or a vow expects two or more arguments, return the error as the first argument and don't report it (let the user handle it); When suite.options.error is set to true (default) and a vow expects zero or one parameters, report the error in the test runner and don't run the vow.

Update:

I fixed another issue (#231) in this pull request: sub-topics will still proceed if the parent topic throws or returns an error.

Update:

Apparently, basic error-handling through vows is also broken(#280)--fixed by this pull request

adamstallard added some commits
@adamstallard adamstallard fix typo in comment 26e5941
@adamstallard adamstallard Handle errors correctly based on suite.options.error and the number o…
…f parameters expected by the vow: When suite.options.error is set to false or a vow expects two or more parameters, get the error as the first argument and don't report it; When suite.options.error is set to true and a vow expects zero or one parameters, report the error and do not run the vow.
50a13b5
@JerrySievert

can you do a merge so that there's a possibility of the merge occurring without conflicts? if so, i will take a look.

thanks much!

@adamstallard

I merged from upstream and resolved the conflicts in lib/vows.js and lib/vows/reporters/spec.js (I chose the upstream over my changes). I ran the tests and they work--except isolate and suppress-stdout are still broken on Windows, node 0.10. (They do work on linux though)

@JerrySievert

hm. i seem to have lost push after the move from @cloudhead to @flatiron - @indexzero can you reinstate?

@adamstallard

@JerrySievert thanks for looking at this again. I haven't merged from upstream on my fork for 9 months, so let me know if you need me to do that.

@adamstallard

@flatiron I'd be willing to be a maintainer of vows as well (of the repo and/or the npm package). I'm pretty familiar with the code and I still use it as my primary js test framework. Also, how is the website maintained? The documentation there is out-of-date.

@adamstallard

Looks like the only thing that's changed since I merged last was adding the repository field to package.json #283 , so this should still be good to merge with no conflicts and I won't both to update my fork just for that.

@JerrySievert JerrySievert merged commit aa8cd5e into vowsjs:master

1 check passed

Details default The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 23, 2013
  1. @adamstallard

    fix typo in comment

    adamstallard committed
  2. @adamstallard

    Handle errors correctly based on suite.options.error and the number o…

    adamstallard committed
    …f parameters expected by the vow: When suite.options.error is set to false or a vow expects two or more parameters, get the error as the first argument and don't report it; When suite.options.error is set to true and a vow expects zero or one parameters, report the error and do not run the vow.
  3. @adamstallard
  4. @adamstallard
Commits on Feb 19, 2013
  1. @adamstallard

    Make the xunit reporter follow the established pattern of using the v…

    adamstallard committed
    …ows console module and allowing for overriding the stream.
Commits on Feb 20, 2013
  1. @adamstallard
Commits on Apr 2, 2013
  1. @adamstallard

    Set the return value to 'undefined' on an unexpected error (since we …

    adamstallard committed
    …always use a callback); improve comments
Commits on Apr 17, 2013
  1. @adamstallard
Commits on Aug 19, 2013
  1. @adamstallard

    update vows package

    adamstallard committed
  2. @adamstallard

    Merge from Upstream

    adamstallard committed
  3. @adamstallard

    remove accidental commit

    adamstallard committed
  4. @adamstallard
  5. @adamstallard
  6. @adamstallard
  7. @adamstallard
  8. @adamstallard
  9. @adamstallard
  10. @adamstallard

    Merge

    adamstallard committed
  11. @adamstallard

    update vows package

    adamstallard committed
  12. @adamstallard
  13. @adamstallard
  14. @adamstallard

    update vows package

    adamstallard committed
  15. @adamstallard

    ignore hg files

    adamstallard committed
Commits on Aug 21, 2013
  1. @adamstallard
Showing with 103 additions and 22 deletions.
  1. +4 −0 .gitignore
  2. +4 −0 lib/vows.js
  3. +1 −1 lib/vows/extras.js
  4. +6 −1 lib/vows/reporters/xunit.js
  5. +14 −9 lib/vows/suite.js
  6. +73 −2 test/vows-error-test.js
  7. +1 −9 test/vows-test.js
View
4 .gitignore
@@ -1,4 +1,8 @@
node_modules
.idea
.nul
+.hgignore
+.hgsubstate
+.hg
+.hgsub
test/npm-debug.log
View
4 lib/vows.js
@@ -64,6 +64,9 @@ function addVow(vow) {
// always set a listener on the event
this.on(event, function () {
+ if(vow.caughtError)
+ return;
+
var args = Array.prototype.slice.call(arguments);
// If the vow is a sub-event then we know it is an
// emitted event. So I don't muck with the arguments
@@ -80,6 +83,7 @@ function addVow(vow) {
if (event !== 'error') {
this.on("error", function (err) {
+ vow.caughtError = true;
if (vow.callback.length >= 2 || !batch.suite.options.error) {
runTest(arguments, this.ctx);
} else {
View
2 lib/vows/extras.js
@@ -1,6 +1,6 @@
var events = require('events');
//
-// Wrap a Node.js style async function into an EventEmmitter
+// Wrap a Node.js style async function into an EventEmitter
//
this.prepare = function (obj, targets) {
targets.forEach(function (target) {
View
7 lib/vows/reporters/xunit.js
@@ -4,7 +4,9 @@
// added, see: http://ant.1045680.n5.nabble.com/schema-for-junit-xml-output-td1375274.html
//
-var puts = require('util').puts;
+var options = { tail: '\n', raw: true };
+var console = require('../../vows/console');
+var puts = console.puts(options);
var buffer = [],
curSubject = null;
@@ -47,6 +49,9 @@ function cdata(data) {
}
this.name = 'xunit';
+this.setStream = function (s) {
+ options.stream = s;
+};
this.report = function (data) {
var event = data[1];
View
23 lib/vows/suite.js
@@ -129,13 +129,14 @@ this.Suite.prototype = new(function () {
}
// Run the topic, passing the previous context topics
- // If topic `throw`s an exception, pass it down as a value
try {
topic = topic.apply(ctx.env, ctx.topics);
}
+ // If an unexpected error occurs in the topic, set the return
+ // value to 'undefined' and call back with the error
catch (ex) {
- if(/ReferenceError/.test(ex)) throw ex;
- topic = ex;
+ ctx.env.callback(ex);
+ topic = undefined;
}
if (typeof(topic) === 'undefined') { ctx._callback = true }
@@ -229,13 +230,17 @@ this.Suite.prototype = new(function () {
if (topic &&
ctx.name !== 'on' &&
(!topic._vowsEmitedEvents || !topic._vowsEmitedEvents.hasOwnProperty(ctx.event))) {
- topic.on(ctx.event, function (ctx) {
- return function (val) {
- return run(new(Context)(vow, ctx, env), lastTopic);
+ var runInnerContext = function(ctx){
+ return function(val){
+ return run(new (Context)(vow, ctx, env), lastTopic);
};
- }(ctx));
- } else {
- run(new(Context)(vow, ctx, env), lastTopic);
+ }(ctx);
+ topic.on(ctx.event, runInnerContext);
+ // Run an inner context if the outer context fails, too.
+ topic.on('error', runInnerContext);
+ }
+ else {
+ run(new (Context)(vow, ctx, env), lastTopic);
}
}
});
View
75 test/vows-error-test.js
@@ -2,7 +2,8 @@ var path = require('path'),
events = require('events'),
assert = require('assert'),
fs = require('fs'),
- vows = require('../lib/vows');
+ vows = require('../lib/vows'),
+ silent = require('../lib/vows/reporters/silent');
function doSomethingAsync(callback) {
var err = null;
@@ -48,4 +49,74 @@ vows.describe('vows/error').addBatch({
assert.equal(testValue, 'a');
}
}
-}).export(module)
+}).export(module)
+
+vows.describe('Error Handling').addBatch({
+ "A topic with a function that errors": {
+ topic: function() {
+ throw("Something wrong here");
+ },
+ "should return an error to a vow with two parameters": function(e, data) {
+ assert.equal(e, "Something wrong here");
+ }
+ },
+ "A topic with a built-in error": {
+ topic: function() {
+ bad.bad;
+ },
+ "should return an error to a vow with two parameters": function(e, data) {
+ assert(e instanceof Error, "Return value " + e + " wasn't an Error.");
+ }
+ },
+ "The previous two topics run in their own suite," : {
+ "connected to two vows expecting one argument each" : {
+ topic: function(){
+ vows.describe().addBatch({
+ "A topic with a function that errors": {
+ topic: function() {
+ throw("Something wrong here");
+ },
+ "should record the error in the test results" : function(data) {
+ assert.ok(!data);
+ }
+ //» An unexpected error was caught: "Something wrong here"
+ },
+ "A topic with a built-in error": {
+ topic: function() {
+ bad.bad;
+ },
+ "should record the error in the test results" : function(data) {
+ assert.ok(!data);
+ }
+ //» An unexpected error was caught: ReferenceError: bad is not defined
+ }
+ }).run({reporter : silent}, this.callback);
+ },
+ "should have an errored count of two" : function(results, unused) {
+ assert.equal(results.errored, 2);
+ },
+ "should have a total count of two" : function(results, unused) {
+ assert.equal(results.total, 2);
+ },
+ "should have an honored count of zero" : function(results, unused){
+ assert.equal(results.honored, 0);
+ }
+ }
+ },
+ "A topic with an error in it" : {
+ topic : function(){
+ throw('awesome');
+ },
+ "should error" : function(error, result){
+ assert.equal(error, 'awesome');
+ },
+ "containing a subtopic" : {
+ topic : function(){
+ return 52;
+ },
+ "should reach a vow in the subtopic" : function(){
+ }
+ }
+ }
+}).export(module);
+
View
10 test/vows-test.js
@@ -168,15 +168,7 @@ vows.describe("Vows").addBatch({
"should work as expected": function (topic) {
assert.isFunction(topic);
assert.equal(topic(), 42);
- },
- }
- },
- "A topic with a function that errors": {
- topic: function() {
- throw("Something wrong here");
- },
- "should error out": function(topic) {
- assert.equal(topic, "Something wrong here");
+ }
}
},
"A topic emitting an error": {
Something went wrong with that request. Please try again.