Ava: Check assertion message is a string

[creeperyang]

(See https://github.com/avajs/ava/issues/1125#issuecomment-264707430, @novemberborn)

---

[email protected], [email protected]

When I run test, sometimes I get this error:

TypeError: test.error.message.split is not a function
    at /Users/creeper/work/projects/ysprite/node_modules/ava/lib/reporters/mini.js:181:43
    at Array.forEach (native)
    at MiniReporter.finish (/Users/creeper/work/projects/ysprite/node_modules/ava/lib/reporters/mini.js:173:20)
    at Logger.finish (/Users/creeper/work/projects/ysprite/node_modules/ava/lib/logger.js:49:27)
    at /Users/creeper/work/projects/ysprite/node_modules/ava/lib/cli.js:161:12
From previous event:
    at Object.exports.run (/Users/creeper/work/projects/ysprite/node_modules/ava/lib/cli.js:160:5)
    at Object.<anonymous> (/Users/creeper/work/projects/ysprite/node_modules/ava/cli.js:23:24)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.runMain (module.js:604:10)
    at run (bootstrap_node.js:394:7)
    at startup (bootstrap_node.js:149:9)
    at bootstrap_node.js:509:3

And I found this maybe caused by:

test('message', t => {
  return aPromise.then(() => {
    // error occured 
  })
})

[sindresorhus]

What error occured in your promise?
The only way the above error can be hit is if the message of the error exists but is not a string.

[creeperyang]

console.log('------>', test.error);

------> { name: 'AssertionError',
  actual: false,
  expected: false,
  operator: 'fail',
  message: 
   { name: 'Error',
     stack: 'Error: ENOENT: no such file or directory, open \'app.png\'\n    at Error (native)',
     message: 'ENOENT: no such file or directory, open \'app.png\'',
     errno: -2,
     code: 'ENOENT',
     syscall: 'open',
     path: 'app.png' },
  generatedMessage: false,
  stack: 'AssertionError: Error: ENOENT: no such file or directory, open \'app.png\'\n    generateSprite.then.then.catch.err (test/sprite.spec.js:103:11)\n    ' }

test.message is an Error object.

[creeperyang]

It eventually looks like caused by calling t.fail(err). I pass an Error object to t.fail and it seems we should use string here.

However I think it's good to support both error message and error object.

x.fail = function (msg) {
        if (msg instanceof Error) {
            msg = msg.message
        }
    msg = msg || 'Test failed via t.fail()';
    test(false, create(false, false, 'fail', msg, x.fail));
};

[novemberborn]

@creeperyang t.fail() only takes a message, just like the other assertions take a message as their last argument.

@avajs/core perhaps it would be useful to typecheck that argument?

[vadimdemedes]

@novemberborn Agree, a check wouldn't hurt.

[sindresorhus]

:+1:

[creeperyang]

👍 much better.

[novemberborn]

Cool. We're now looking to add a typecheck for the optional assertion message parameter, ensuring it's a string if it is provided.

[leewaygroups]

May I take on this as a gentle step into AVA.

One question: For uniformity, would it not make sense to apply same check in other sibling functions such truthy, falsy etc?

In that case the check can be centralised as a helper and invoked in all sibling functions where it is needed.

[novemberborn]

@leewaygroups yes please! Thank you!

[novemberborn]

To clarify, per https://github.com/avajs/ava/pull/1831#issuecomment-396913731, we want to type check the message parameter when the assertion is invoked, and fail the assertion if the parameter is not undefined and not a string.