WIP: Update .throw and add .error

[meeber]

DO NOT MERGE; THIS IS A WORK IN PROGRESS!!

(Note: I'm recreating this PR because I accidentally closed #1015 and don't seem able to re-open it.)

I've been doing a lot of work on #930. This turned out to be a lot harder than expected; .throw couldn't be updated to simply call .error internally due to the difference in failed assertion messages between the two. I suspect this is the same kind of issue causing problems in plugins like chai-as-promised when they try to adapt .throw.

The more I looked at our current .throw implementation, the more I realized that the complexity wasn't because of negation as we originally thought but rather because of how the failed assertion messages were being composed. As a result, this PR turned into a pretty big refactor of .throw to reduce complexity. This also required changing some of .throw's failed assertion messages to be more consistent.

Some notes/thoughts:

• This PR currently only adds .error to the BDD interface. I'll add the Assert interface later.
• Is changing a failed assertion message considered a breaking change? On the one hand, it's likely to break the tests of any plugins that do string matching against failed assertion messages. On the other hand, it's unlikely to break the actual functionality of the plugin from the end user's perspective, or any of the end user's tests. I'm currently learning toward "no".
• The matchError and decribeExpectedError helper functions should probably be moved to the check-error module.
• Part of what .throw does is assert that a function throws, without taking into account what it throws. This means it will work if the target function throws something besides an Error object (e.g., a string, a number, null, or even undefined). I think it's important that the .throw logic retain this base functionality. However, when it comes to adding arguments to the .throw function to perform additional assertions on the value that was thrown, I think we should consider requiring that the object thrown be an Error object. Currently, the behavior of the various check-error functions is a little sketchy if the object being tested isn't an Error. This problem carries over into the matchError helper function as well.

[pjanuario]

This will definitely improve test readability and give better output to failing tests. I am currently migrating a microservice infra too a new environment and migrating the CD pipeline too and I am having errors such as: Uncaught AssertionError: expected { Object (cause, isOperational) } to not exist while using should.not.exist(err);

Thanks for your effort.

[codecov]

Codecov Report

Merging #1021 into master will increase coverage by 0.89%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1021      +/-   ##
==========================================
+ Coverage   93.69%   94.58%   +0.89%     
==========================================
  Files          32       32              
  Lines        1649     1625      -24     
  Branches      396      374      -22     
==========================================
- Hits         1545     1537       -8     
+ Misses        104       88      -16

Impacted Files	Coverage Δ
lib/chai/interface/assert.js	89.75% <ø> (-0.17%)	⬇️
lib/chai/core/assertions.js	99.67% <100%> (+0.3%)	⬆️
lib/chai/utils/inspect.js	82.7% <0%> (+0.54%)	⬆️
lib/chai/utils/expectTypes.js	100% <0%> (+76.47%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e354bee...aa886d2. Read the comment docs.

[meeber]

I just opened chaijs/check-error#18, and then updated this PR to use that. This is still in a WIP state; it's still missing the assert interface updates, and it's loading check-error from that branch on github.

This is now definitely a breaking change. In addition to making small adjustments to failed assertion messages (for consistency and simplicity), it also no longer allows .throw to be used to assert details about anything other than Error instances. (As noted in the update docs, Non-Error objects can still be asserted on by chaining together assertions.)

This also resolves #1070 (and closes #1071).

Also related: chaijs/chai-as-promised#235

[bryanbraun]

Hey I was going to update #1070 tonight but I saw this work happening. Kudos for taking on the refactor @meeber. If you'd like, I can hold off on making updates on #1070 so you can focus on this more comprehensive approach.

[keithamus]

@meeber we're clearing house on the open PRs and issues (as discussed on the team discussion). As this has been a WIP PR open for a while, we're going to close this now - just for housekeeping.

@lucasfcosta and I are working on Chai together. We've added https://github.com/chaijs/chai/projects/2 which will cover this so we can track it through to Chai 5.

Please don't delete your branch! We'll pick it up and run with it hopefully in the next few days!