Rework error reporting and formatting

[gavv]

Problem

Currently we're missing the following features:

• the user can't register a hook that will be invoked on every performed assertion including successful ones; e.g. this could be used for goconvey integration (How integrate with goconvey? #143) or for other reasons;

• when an error is passed to user-defined Reporter, it only receives the error message, but no additional context: e.g. test name (#59), request and response (printing body when unexpected status code #137), value which didn't pass the assertion, etc;

• since every component formats error message by its own, there is no single point where we can implement various formatting improvements like colored output.

Solution

The suggested solution is to unify error handling by introducing error context and formatter:

• Add Context struct:

  • Context will accumulate the assertion context, assertion name (i.e. method name like "Equal"), request, response, round trip time, etc. E.g., when the user calls Expect(), the encoded request and received response are stored into context.

  • Context should be inherited by nested objects, e.g. in Array().Element() the returned element inherits the context of the array.

  • Context will be passed to Formatter when an assertion fails.

  • Context will contain Reporter field, which will be used to report failures.

  • Context will also contain private Formatter field, to allow nested components to inherit formatter.

  • We already have struct chain which is automatically inherited by all nested objects. We should just replace chain.reporter with chain.context.

• Add Failure struct:

  • Failure will contain information about the failed assertion: error message, actual value, and expected value (for assertions like Equal) or the value which triggered the failure (for assertions like Contains).

  • Failure will be passed to Formatter when an assertion fails.

• Add Formatter interface with four methods:

  BeginAssertion(Context)
  Success(Context)
  Failure(Context, Failure)
  EndAssertion(Context)
  

• Add DefaultFormatter implementation of the Formatter interface:

  • BeginAssertion(), EndAssertion(), and Success() may be no-op for now.

  • Failure() should format info from Context and Failure structs into a string and pass it to Context.Reporter.

  • It should use dumpValue() to print values from Failure struct. If both actual and expected values are present, it should use diffValues() to print the diff. Currently this logic is duplicated in every component and now it should be implemented once in Formatter.

• Switch all components to new structs:

  • Make all components using Context.

  • Add Formatter to Config. If it's nil, automatically use DefaultFormatter. Pass Formatter and Reporter from Config to to Context.

  • Incrementally, one-by-one, switch every component from doing its own error formatting and reporting to using Context.Formatter. They should call BeginAssertion() before doing a check, then call Success() and Failure(), and then call EndAssertion().

Notes

Since these are quite large changes, it's better to split the implementation into multiple PRs, like:

1. Add Context, Failure, and Formatter types.
2. Add DefaultFormatter.
3. Use Context in all components.
4. Use Formatter in every component (this step can be split into multiple PRs as well, e.g. one PR for one or a few components).

Preferably we should not break public API. For now it seems that we can do it. Otherwise we'll have to make v3 branch.

Further improvements

After finishing this issue, we'll be able to implement the following improvements:

• allow to assign a name to every test and then print it on failure;
• pretty-print request, response, and RTT on failure;
• print backtrace on failure;
• colored output, including colored diff.

When this task is done, we'll create new issues for these features.

[goku321]

@gavv Can you please assign this issue to me? I will start working on this. Thanks

[goku321]

Hi @gavv I will be needing your help while working on this, so please bear with me 😅 . I'll create a PR soon (and it might not be completely correct), to get your inputs. I'll make sure I complete this ASAP.

[goku321]

@gavv A question: why not pointer to Context and Failure structs for Failure method of Formatter interface? Also, would it make sense to keep Failure struct inside Context?

[gavv]

why not pointer to Context and Failure structs for Failure method of Formatter interface?

It was just a pseudo-code. We can use pointers of course if we need to.

[gavv]

Also, would it make sense to keep Failure struct inside Context?

What do you mean by "inside"?

[gavv]

What do you mean by "inside"?

Ah, I guess you suggest to add a Failure field to Context. Well, I think that Formatter interface will be cleaner if context and failure are distinguished: Context defines where the error happened, and Failure defines what error happened. But if there are reasons why it's better to move Failure into Context, we can do it.

[goku321]

It makes sense, @gavv . Thanks for the explanation.

[fpeterschmitt]

@gavv about not breaking the public API, i think there is a problem with the NewNumber NewObject (and so on…) constructors: since we need to pass the context instead, that will break any code that relies on those functions.

Do you consider this to be part of the public API, or by public API you mean only the Expect struct?

[fpeterschmitt]

I think that the Formatted should only be called by a Reporter, as it passes the message to the backend, knowing when to call (with current implementation of Formatter) the different functions of a Formatter, with a Failure struct.

However that means reworking a lot of API, surely breaking some of the public ones.

The Expect struct implem should not change though.

[fpeterschmitt]

See comment #98 (comment)

[gavv]

This is finally landed in #150. I'll update README soon.

[gavv]

See two new sections in README:

• https://github.com/gavv/httpexpect#customize-failure-formatting
• https://github.com/gavv/httpexpect#customize-assertion-handling

Closing this. I'll create separated issues for further improvements.

[gavv]

Tagged v2.5.0 with this feature