From ec0442ccd04187a88bc52fcb1386dc03827ac8e8 Mon Sep 17 00:00:00 2001 From: James Gibson Date: Wed, 5 Nov 2014 14:40:21 -0500 Subject: [PATCH 1/2] Make assert.epsilon fail for NaN actual value It makes sense than NaN would not be within a tolerance of a number. but since Math.abs(NaN) = NaN and NaN > x evalutes to false for all numeric x, assert.epsilon will pass if NaN is passed as the 'actual' argument. I could not determine how to use vows to assert that an assertion would fail, so there is no test case for this change, but the existing tests pass. I'm not sure asserting that an assertion will fail is possible with the library's promise-oriented design. This bug was originally found while working on the jStat project which relies on vows; if the vows team does not want to make assert.epsilon fail for NaN actual value, it would be good to have an additional method that would check both that the argument is a number and within the tolerance. --- lib/assert/macros.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/assert/macros.js b/lib/assert/macros.js index 0fa2ee4..29ea7d3 100644 --- a/lib/assert/macros.js +++ b/lib/assert/macros.js @@ -17,7 +17,7 @@ for (var key in messages) { } assert.epsilon = function (eps, actual, expected, message) { - if (Math.abs(actual - expected) > eps) { + if (isNaN(actual) || Math.abs(actual - expected) > eps) { assert.fail(actual, expected, message || "expected {expected} \u00B1"+ eps +", but was {actual}"); } }; From f5cec76be4041c48dd868ee512eec538b434e0f8 Mon Sep 17 00:00:00 2001 From: James Gibson Date: Wed, 5 Nov 2014 19:11:12 -0500 Subject: [PATCH 2/2] Tests for assert.epsilon failing on NaN Tests to make sure assert.epsilon fails when either the epsilon or actual arguments are NaN. Added a different failure message for the case that epsilon is NaN. --- lib/assert/macros.js | 4 +++- test/assert-test.js | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/assert/macros.js b/lib/assert/macros.js index 29ea7d3..c3b675e 100644 --- a/lib/assert/macros.js +++ b/lib/assert/macros.js @@ -17,7 +17,9 @@ for (var key in messages) { } assert.epsilon = function (eps, actual, expected, message) { - if (isNaN(actual) || Math.abs(actual - expected) > eps) { + if (isNaN(eps)) { + assert.fail(actual, expected, message || "cannot compare {actual} with {expected} \u00B1 NaN"); + } else if (isNaN(actual) || Math.abs(actual - expected) > eps) { assert.fail(actual, expected, message || "expected {expected} \u00B1"+ eps +", but was {actual}"); } }; diff --git a/test/assert-test.js b/test/assert-test.js index 8d41267..fb30eb2 100644 --- a/test/assert-test.js +++ b/test/assert-test.js @@ -9,6 +9,12 @@ vows.describe('vows/assert').addBatch({ }, "`epsilon`": function() { assert.epsilon(1e-5, 0.1+0.2, 0.3); + assert.throws(function() { + assert.epsilon(1e-5, NaN, 0.3); + }); + assert.throws(function() { + assert.epsilon(NaN, 1.0, 1.0); + }); }, "`match`": function () { assert.match("hello world", /^[a-z]+ [a-z]+$/);