chaijs / chai

BDD / TDD assertion framework for node.js and the browser that can be paired with any testing framework.
https://chaijs.github.io
MIT License
8.15k stars 698 forks source link

Update `assertion-error` to it's latest major version! #1543

Closed koddsson closed 1 year ago

koddsson commented 1 year ago

assertion-error is now a ES Module.

koddsson commented 1 year ago

@keithamus does this failure look familiar at all?

  !

  0 passing (6ms)
  1 failing

  1) should
       frozen:
     AssertionError: implementation frames not properly filtered from stack trace: expected 'AssertionError: expected {} to be fro…' not to match /at [a-zA-Z]*(Getter|Wrapper|(\.)*assert)/
      at Assertion.assert (file:///Users/cr91ew/src/koddsson/chai/chai.js:1651:11)
      at Proxy.assertMatch (file:///Users/cr91ew/src/koddsson/chai/chai.js:2752:8)
      at Proxy.methodWrapper (file:///Users/cr91ew/src/koddsson/chai/chai.js:1814:25)
      at globalErr (file:///Users/cr91ew/src/koddsson/chai/test/bootstrap/index.js:42:23)
      at Context.<anonymous> (file:///Users/cr91ew/src/koddsson/chai/test/should.js:3267:5)
      at process.processImmediate (node:internal/timers:478:21)

The stacktrace for the error in question looks like this:

AssertionError: expected {} to be frozen
    at Assertion.assert (file:///Users/cr91ew/src/koddsson/chai/chai.js:1651:11)
    at Assertion.<anonymous> (file:///Users/cr91ew/src/koddsson/chai/chai.js:3251:8)
    at Assertion.propertyGetter (file:///Users/cr91ew/src/koddsson/chai/chai.js:1689:29)
    at Reflect.get (<anonymous>)
    at Object.proxyGetter [as get] (file:///Users/cr91ew/src/koddsson/chai/chai.js:1773:22)
    at file:///Users/cr91ew/src/koddsson/chai/test/should.js:3268:24
    at globalErr (file:///Users/cr91ew/src/koddsson/chai/test/bootstrap/index.js:37:5)
    at Context.<anonymous> (file:///Users/cr91ew/src/koddsson/chai/test/should.js:3267:5)
    at callFn (/Users/cr91ew/src/koddsson/chai/node_modules/mocha/lib/runnable.js:366:21)
    at Runnable.run (/Users/cr91ew/src/koddsson/chai/node_modules/mocha/lib/runnable.js:354:5)
    at Runner.runTest (/Users/cr91ew/src/koddsson/chai/node_modules/mocha/lib/runner.js:677:10)
    at /Users/cr91ew/src/koddsson/chai/node_modules/mocha/lib/runner.js:801:12
    at next (/Users/cr91ew/src/koddsson/chai/node_modules/mocha/lib/runner.js:594:14)
    at /Users/cr91ew/src/koddsson/chai/node_modules/mocha/lib/runner.js:604:7
    at next (/Users/cr91ew/src/koddsson/chai/node_modules/mocha/lib/runner.js:486:14)
    at Immediate._onImmediate (/Users/cr91ew/src/koddsson/chai/node_modules/mocha/lib/runner.js:572:5)
    at process.processImmediate (node:internal/timers:478:21) 
keithamus commented 1 year ago

yeah AssertionError is failing to properly remove stack frames for some reason. We have code that elides stack trace lines from Chai itself, so you can see your own test stack frames more clearly.

https://github.com/chaijs/chai/blob/744a16e1cc4e8a9c6d4499e1e520a0bc4c80ec18/test/bootstrap/index.js#L19-L21

koddsson commented 1 year ago

yeah AssertionError is failing to properly remove stack frames for some reason. We have code that elides stack trace lines from Chai itself, so you can see your own test stack frames more clearly.

Ok cool that jives with what I was looking at. Looking at the difference between assertion-error v1 and v2 it wasn't obvious what we changed here. I'll have to give this another look.

koddsson commented 1 year ago

It looks like chai previously treated AssertionError as a object and not a error but now it correctly identifies it as a error so loupe sets the message differently. Do we wanna try to keep the existing output or change the test?

koddsson commented 1 year ago

What do you think @keithamus ?

koddsson commented 1 year ago

Hmm, no this should work 🤔

koddsson commented 1 year ago

Reading the loupe code this is working as intended so I'm gonna update this test.