Bring calledWith messages more in line with Sinon's output #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a continuation in the spirit of chaijs#104, but resolves the awkwardness with the negative version, and brings us closer to core Sinon messages. To do this, we changed the nonNegatedSuffix parameter to instead take a pair of suffixes, nonNegated and negated. This allows us to specify %D for the nonNegated case, which eliminates noisy duplication, while still using %*%C for the negated case. This also matches what core Sinon outputs in these cases. As shorthand, we still support specifying a single value, which we use for nonNegatedSuffix and assume an empty negatedSuffix.
I'm moderately happy with this. It got a little messy because now we can't depend on the brief but duplicative listing of arguments, and have to match against the diff view, which is colorized and has newlines and things. I pulled in strip-ansi@6 because it's already a transitive dependency, and added a little wrapper function to rewrite the error messages being thrown. The wrapper is a little weird, but was the best solution I came up with. Other things I tried or considered: - Disable color output (only possible by manipulating process.env or process.argv, both of which seem like a no-go) - Create a custom matcher that strips the message before comparing (throw() doesn't support matchers) - Matching color escapes with a RegExp (seemed way messier than this) - Manually try/catch, store error, assert against the error (also seemed messier than this) The wrapper could be elsewhere too - it could be a wrapper that calls expect(), for instance - but I think this is clear enough.
This was broken before this PR, but might as well fix it
I discovered this bug while working on this, there's a PR for fix here for reference: sinonjs/sinon#2407
This is a bit of a regression imo on Sinon's part, but I don't see an easy way to adjust for it
cincodenada
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a local version of chaijs#152