Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrumenting assert.throws() #39

Closed
feugy opened this issue Dec 23, 2017 · 2 comments
Closed

Instrumenting assert.throws() #39

feugy opened this issue Dec 23, 2017 · 2 comments

Comments

@feugy
Copy link
Contributor

feugy commented Dec 23, 2017

Hello!

We've been using power-assert with Lab (and Ava to some extend) since a while, and it's really a wonderful tool.
One of the few missing bits is the instrumentation of assert.throw().

Before even trying to contribute, I'd like first to validate with you that I'm taking the proper approach.
After few experiments, here is what I came up with.

Case 1

assert.throws(function() {/* whatever */});

Could be turned into:

var _rec1 = new _PowerAssertRecorder1();
_rec1.hasThrown = false;
try {
  (function() {/* whatever */})();
} catch (err) {
  _rec1.hasThrown = true;
}
// check it has thrown
assert(_rec1._expr(_rec1.hasThrown? true : !_rec1._capt('did not throw', 'arguments/0'), {
  content: 'assert.throws(function() {/* whatever */});',
  filepath: 'path/to/some_test.js',
  line: 1
})/* , message*/);

When the asserted function doesn't throw, power-assert will report the following:

AssertionError [ERR_ASSERTION]:   # path/to/some_test.js:1

  assert.throws(function() {/* whatever */});
                |
                "did not throw"

assert.throws(block, [expected], [message]) also have a message argument which, if present, should be reported when the asserted didn't throw.

Case 2

assert.throws(function() {/* whatever */}, RangeError);

Could be turned into:

_rec1.hasThrown = false;
try {
  (function() {/* whatever */})();
} catch (err) {
  _rec1.hasThrown = true;
  // validates expected class
  assert(_rec1._expr(_rec1._capt(err instanceof RangeError, 'arguments/1') || !_rec1._capt(err.toString(), 'callee'), {
    content: 'assert.throws(function() {/* whatever */}, RangeError);',
    filepath: 'path/to/some_test.js',
    line: 1
  }));
}
// check it has thrown
assert(_rec1._expr(_rec1.hasThrown? true : !_rec1._capt('did not throw', 'arguments/0'), {
  content: 'assert.throws(function() {/* whatever */}, RangeError);',
  filepath: 'path/to/some_test.js',
  line: 1
})/*, message */);

When the asserted function thrown object with unexpected class, power-assert will report the following:

AssertionError [ERR_ASSERTION]:   # path/to/some_test.js:1

  assert.throws(function() {/* whatever */}, RangeError);
         |                                   |
         "Error: unexpected"                 false

Case 3

assert.throws(function() {/* whatever */}, /expected/);

Could be turned into:

var _rec1 = new _PowerAssertRecorder1();
_rec1.hasThrown = false;
try {
  (function() {/* whatever */})();
} catch (err) {
  _rec1.hasThrown = true;
  // validates expected regexp
  assert(_rec1._expr(_rec1._capt(/expected/.test(err), 'arguments/1') || !_rec1._capt(err.toString(), 'callee'), {
    content: 'assert.throws(function() {/* whatever */}, /expected/);',
    filepath: 'path/to/some_test.js',
    line: 1
  }));
}
// check it has thrown
assert(_rec1._expr(_rec1.hasThrown? true : !_rec1._capt('did not throw', 'arguments/0'), {
  content: 'assert.throws(function() {/* whatever */}, /expected/);',
  filepath: 'path/to/some_test.js',
  line: 1
})/* , message*/);

When the asserted function thrown value which doesn't match regexp, power-assert will report the following:

AssertionError [ERR_ASSERTION]:   # path/to/some_test.js:1 
                                                           
  assert.throws(function() {/* whatever */}, /expected/);  
         |                                   |             
         "10"                                /expected/    

Case 4

assert.throws(function() {/* whatever */}, err => err instanceof Error && err.message.includes('expected'));

Could be turned into:

var _rec1 = new _PowerAssertRecorder1();
_rec1.hasThrown = false;
try {
  (function() {/* whatever */})();
} catch (err) {
  _rec1.hasThrown = true;
  // validates expected function
  assert(_rec1._expr(_rec1._capt((err => err instanceof Error && err.message.includes('expected'))(err), 'arguments/1') || !_rec1._capt(err.toString(), 'callee'), {
    content: 'assert.throws(function() {/* whatever */}, err => err instanceof Error && err.message.includes(\'expected\'));',
    filepath: 'path/to/some_test.js',
    line: 1
  }));
}
// check it has thrown
assert(_rec1._expr(_rec1.hasThrown? true : !_rec1._capt('did not throw', 'arguments/0'), {
  content: 'assert.throws(function() {/* whatever */}, err => err instanceof Error && err.message.includes(\'expected\'));',
  filepath: 'path/to/some_test.js',
  line: 1
})/* , message*/);

When the asserted function thrown value that doesn't pass custom function, power-assert will report the following:

AssertionError [ERR_ASSERTION]:   # path/to/some_test.js:1

  assert.throws(function() {/* whatever */}, err => err instanceof Error && err.message.includes('expected'));
         |                                   |
         "SyntaxError: wrong"                false

Case 5

For doesNotThrow(), it could be slightly simpler:

assert.doesNotThrow(function() {/* whatever */});

Could be turned into:

try {
  (function() {/* whatever */ })();
} catch (err) {
  assert(_rec1._expr(err && !_rec1._capt('threw ' + err, 'arguments/0'), {
    content: 'assert.doesNotThrow(function() {/* whatever */});',
    filepath: 'path/to/some_test.js',
    line: 1
  })/*, message */);
}

Which will produce on unexpected error:

AssertionError [ERR_ASSERTION]:   # path/to/some_test.js:1

  assert.doesNotThrow(function() {/* whatever */});
                      |
                      "threw Error: unexpected"

For the expected second parameter, I'm not sure it worth any instrumentation, as the result will be logically the same.
As soon as the asserted function throws something, whether it comply with expected class/regexp/custom function or not, an AssertionError will be raised.


That's it, thanks for your time and feedback!

@twada
Copy link
Member

twada commented Dec 25, 2017

@feugy Thank you for your comments! Instrumenting assert.throws is a bit difficult task to wrestle with, There may be some edge cases to consider carefully. I'll review your implementation plans and also show you what's I'm thinking. Let's make it happen together.

@feugy
Copy link
Contributor Author

feugy commented Mar 17, 2018

Hello @twada. Any news on this?

You might be interested to know that rejects() and doesNotReject() were added to nodejs assert module.

Having those 4 functions instrumented would be really awesome.

@feugy feugy closed this as completed Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants