MithrilJS / ospec

Noiseless testing framework
MIT License
48 stars 13 forks source link

'throws' doesn't work with async functions #33

Open vitoreiji opened 3 years ago

vitoreiji commented 3 years ago

ospec version: 4.1.1 node version: 14.15.4

throws does not seem to catch exceptions from async functions.

Code

o("synchronous", function() {
    o(() => {throw new Error()}).throws(Error)
})
o("asynchronous", function() {
    o(async () => {throw new Error()}).throws(Error)
})

Expected behavior

––––––
All 2 assertions passed (old style total: 2) 

Current behavior

asynchronous:
[AsyncFunction (anonymous)]
  should throw a
[Function: Error] { stackTraceLimit: 10 }
    at /home/vis/dev/repositories/2020-04-pqmail-prototype/report.js:6:40

––––––
1 out of 2 assertions failed (old style total: 2)  
dead-claudia commented 3 years ago

ospec will see a promise rejection, not a thrown exception, and you can't implicitly await within functions. (If you could, this wouldn't even be needed - just do the await within the inner function if you need to await a promise.)

I'll consider this a feature request/gotcha, not a bug.

vitoreiji commented 3 years ago

Then let me add some motivation to this feature request :)

Currently, the best way I know to assert that a given error is thrown is like this:

let error
try{
   await myFunction()
}catch(e){
    error = e
}
o(error.constructor).equals(MyError)

which looks really similar to the code that motivated the PR for throws/notThrows in the first place, except that it has an await.

Of course, if we don't want to use async/await, then the code would look more like this

let error
myFunction().catch(e => {
   error = e
}).finally(() => {
   o(error.constructor).equals(MyError)
   done()
})

I'd argue that this looks even more convoluted. Compare to

o(myFunction).throws(MyError)
// or even
o(async () => myFunction(foo)).throws(MyError)

These look much more readable to me.

Basically, the absence of this feature forces async/await users to revert back to the old style, which MithrilJS/mithril.js#2255 has established is not great.

pygy commented 2 years ago

Hello @vitoreiji this may prove useful.

asyncNotThrows = function(x) {
  return new Promise((f)=>{
    x()
    .then(
      v=>o().equals(), 
      e => o().satisfies(()=>({
      pass: false,
      message: 
`${x.toString()} shouldn't have thrown

${o.cleanStackTrace(e)}
`
    })))
    .finally(f)
  })
}

o("My async test", async function(){
  await asyncNotThrows(async ()=>{throw new Error("Caught by ospec !")})
})
pygy commented 2 years ago

The .satisfies() hook could be extended to return a promise, optionally.