Skip to content

test: add common.mustSucceed

Our increasingly large collection of tests makes frequent use of this pattern:

someFunction(/* some args */, common.mustCall((err, /* some results */) => {
  assert.ifError(err);
  // Something else
});

This PR changes those to this pattern:

someFunction(/* some args */, common.mustSucceed((/* some results */) => {
  // Something else
});

This simple change allows us to remove more than 300 lines of code from tests. Another aspect is that many tests really should be using mustSucceed() instead of mustCall(), because mustCall() silently ignores errors passed to it. I changed the crypto tests manually where I found such cases, and a few other test files, but there's probably dozens or hundreds of such cases in other test files.


To make this easier to review, I separated the changes into multiple commits.

  1. The first commit introduces the new API.
  2. The next three commits apply regular expressions for automatic conversions from mustCall to mustSucceed.
    1. The first replaces all occurrences of mustCall\([ \n]*(\(erro?r?\)[ \n]*=>[ \n]*\{?|function[ \n]*\(erro?r?\)[ \n]*\{)?[ \n]*assert\.ifError(\(erro?r?\))?;?[ \n]*\}?[ \n]*\) with mustSucceed(). These are trivial cases, e.g, mustCall(assert.ifError), mustCall((err) => assert.ifError(err)), mustCall(function(err) { assert.ifError(err); }) etc.
    2. The second replaces all occurrences of mustCall\([\n ]*\(erro?r?(,[\n ]*([^\)]*))?\)([\n ]*)=>([\n ]*)\{[\n ]+assert\.ifError\(erro?r?\);\n with mustSucceed(($2)$3=>$4{\n. These are arrow functions that take an error argument and begin with assert.ifError.
    3. The third replaces all occurrences of mustCall\([\n ]*function[ \n]*\(erro?r?(, *([^\)]*))?\)([ \t\n]*)\{[\n ]+assert\.ifError\(erro?r?\);\n with mustSucceed(function($2)$3{\n. These are regular functions that take an error argument and begin with assert.ifError.
  3. The fifth commit converts functions containing statements such as assert.ok(!err) to mustSucceed, where possible.
  4. The sixth commit improves formatting. While all previous commits pass tests and linter, the regular expressions leave some unnecessary empty lines. While at it, I also converted some functions to arrow functions (only in lines that I changed anyway).
  5. The seventh commit replaces the weaker mustCall with mustSucceed where it seems to make sense.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Merge request reports

Loading