Adds an assertion assertEOSErrorIncludesMessage

[dallasjohnson]

This should assert the error message for an expected eosio_assert_message_exception includes the given description message.

• The purpose of this is to allow more thorough testing of expected errors and ensure that one error is not masked by another error and potentially leaving logical branches of code untested.
• It is also intended to check the error message is included by the thrown error message rather than equals which allows for as error message to be safely changed in the contract while maintaining a constant key (possibly a localised key used by a client.)

eg. A contract may include:

check(existing == statstable.end(), "ERR::CREATE_EXISITNG_SYMBOL::token with symbol already exists");

and a safe test assertion may only check for the ERR::CREATE_EXISITNG_SYMBOL part which allows for the contract developer to safely evolve the error message for debugging purposes without breaking the tests.

[MitchPierias]

Love this idea, and actually have it integrated locally. Consider factoring it directly into the assertEOSError method with a simple determiner within;

if (error.json && error.json.error && error.json.error.name) {
    // Compare error and fail if the error doesn't match the expected
    assert(
      error.json.error.name === eosErrorName,
      `Expected ${eosErrorName}, got ${error.json.error.name} instead.`
    );
    // Check the error details contain the optional message
    if (withMessage && error.json.error.details.length > 0) {
      assert.deepEqual(error.json.error.details[0], {
       message: `assertion failure with message: ${withMessage}`,
      });
    }
    return;
}

Then adding a withMessage argument like so;

export const assertEOSError = async (operation: Promise<any>, eosErrorName:string, withMessage?: string) {}

You can also add some icing to the default assert like;

// Fail if no exception thrown
assert.fail(`Expected ${withMessage || 'error'} but operation completed successfully.`);

Thoughts?

[dallasjohnson]

I was thinking that having a specific assert would mean that the èosErrorNameparam could be omitted from the call and be internally hardcoded since for custom errors generated viacheck(...)` in a contract they are always the same AFAIK.

[MitchPierias]

Ahh yes I see what your going for there! Good idea, much cleaner! I think I ran into this problem too. Can I suggest you add this determiner to assertEOSError and then simplify your assertEOSErrorIncludesMessage(operation:Promimse<any>, withMessage:string)
to simply call assertEOSError with the message and hard coded eosErrorName;

export const assertEOSErrorMessage = async (operation: Promise<any>, message: string) => {
    // Pass our operation and message onto assertEOSError
    return assertEOSError(operation, 'eosio_assert_message_exception', message);
}

This will minimize duplicate code. Also please add yourself to the authors on Kevin's assertEOSError as well if you go with this option

Am I still missing things?

[dallasjohnson]

I had deliberately not checked for equals on the message but instead check the message includes the error. This allows for partial checking of the error string as a showed in the example above.What are your thoughts on that?

[MitchPierias]

I agree, great idea! Would it be a good idea to consolidate your changes into assertEOSError and make assertEOSErrorMessage be a shorthand method, or no? @thekevinbrown thoughts too please?

[thekevinbrown]

I feel like we can be consistent on how we assert EOS errors and this can be a flavour of an EOS error we assert (e.g. shorthand like you're saying)

[thekevinbrown]

A couple of minor things I noticed. Thanks again for the contribution!

Comments resolved