1

I have following:

it('invalid use', () => {
  Matcher(1).case(1, () => {});
});

The case method is supposed to throw after some delay, how can I describe it for Mocha/Chai that's what I want - the test should pass (and must not pass when exception is not thrown)?

Consider case method off limits, it cannot be changed.

For testing purposes it should be equivalent to:

it('setTimeout throw', _ => {
  setTimeout(() => { throw new Error(); }, 1); // this is given, cannot be modified
});

I tried:

it('invalid use', done => {
  Matcher(1).case(1, () => {});
  // calls done callback after 'case' may throw
  setTimeout(() => done(), MatcherConfig.execCheckTimeout + 10);
});

But that's not really helping me, because the test behavior is exactly reverted - when an exception from case (setTimeout) is not thrown, it passes (should fail) and when an exception is thrown the test fails (should succeed).

I read somewhere someone mentioning global error handler, but I would like to solve this cleanly using Mocha and/or Chai, if it is possible (I guess Mocha is already using it in some way).

menfon
  • 1,587
  • 1
  • 11
  • 28

4 Answers4

1

You cannot handle exceptions from within a asynchronous callback, e.g. see Handle error from setTimeout. This has to do with the execution model ECMAScript uses. I suppose the only way to catch it is in fact to employ some environment-specific global error handling, e.g. process.on('uncaughtException', ...) in Node.js.

If you convert your function to Promises, however, you can easily test it using the Chai plugin chai-as-promsied:

import * as chai from 'chai';

import chaiAsPromised = require('chai-as-promised');
chai.use(chaiAsPromised);
const expect = chai.expect;

it('invalid use', async () => {
  await expect(Matcher(1).case(1, () => {})).to.eventually.be.rejected;
});
ComFreek
  • 29,044
  • 18
  • 104
  • 156
  • As I wrote, it cannot be converted to future/async, because it returns another object on which user can operate (chaining `case` calls). The exception from `case` is only for debugging reasons (for library user) and I cannot sacrifice good API just because of a test. – menfon Mar 18 '18 at 13:22
  • @menfon An API which cannot be tested is not a good API. This not only about testing, but users cannot catch exceptions in a simple way either. What about having an `.end()` part at the end of the `case` chain which returns a Promise? – ComFreek Mar 18 '18 at 14:45
  • Users are not supposed to catch that exception. The whole point of this test is to make sure "end" is called - if not not, user can enabled debug mode (e.g. when not in production build) and it will crash the app unless "end" is called after some time after last case is built. It is NOT meant to be visible from API. – menfon Mar 19 '18 at 05:43
0

Any Mocha statements like before, after or it will work asynchronously if you return a promise. I generally use something like the below for async tests. Also don't forget to set timeout this.timeout(...) if you expect the async function to take more than 2 seconds.

it('some test', () => {
    return new Promise(function(resolve,reject){
        SomeAsyncFunction(function(error,vals) {
            if(error) {
                 return reject(error);    
            } else {
                try {
                    //do some chai tests here
                } catch(e) {
                    return reject(e);
                }
                return resolve();
            }
        });
    });
});

Specifically for your case, since we expect some error to be thrown after a period of time (assuming the empty callback you have provided to .case should not run due to the exception being thrown) then you can write it something like:

it('invalid use', () => {
    //define the promise to run the async function
    let prom = new Promise(function(resolve,reject){
        //reject the promise if the function does not throw an error
        //note I am assuming that the callback won't run if the error is thrown
        //also note this error will be passed to prom.catch so need to do some test to make sure it's not the error you are looking for.
        Matcher(1).case(1, () => {return reject(new Error('did not throw'))});
    });
    prom.catch(function(err){
        try {
            expect(err).to.be.an('error');
            expect(err.message).to.not.equal('did not throw');
            //more checks to see if err is the error you are looking for
        } catch(e) {
            //err was not the error you were looking for
            return Promise.reject(e);
        }
        //tests passed
        return Promise.resolve();
    });
    //since it() receives a promise as a return value it will pass or fail the test based on the promise.
    return prom;
});
serakfalcon
  • 3,501
  • 1
  • 22
  • 33
  • I have [tried](https://github.com/mnn/ts-matcher/blob/e37462da24225b3b076cc426caeed8d4bea6a8ac/test/Matcher.ts#L202) your approach, but I couldn't make it work. The future seems to not catch the exception from setTimeout (part in `prom.catch` is never called). It gives same result as the attempt without future - crashes with uncaught exception from setTimeout. – menfon Mar 18 '18 at 13:59
  • @menfon OK so basically I did a little bit more homework. The fundamental problem stems from the fact that you are throwing an error (which is a synchronous way of handling problems) in an async function. The general best practice is to pass the error back to a callback or through a rejected Promise. However, code in the `.then` or `.catch` blocks will convert an uncaught error to a promise rejection. (I thought it would work in the Promise body (I haven't tried before as an uncaught thrown error in async code for me means I didn't structure the underlying function right) but apparently not... – serakfalcon Mar 19 '18 at 02:16
  • 1
    ComFreek's answer is the most correct given the current structure of your code. In order to catch and 'promisify' the thrown synchronous exception, it needs to happen in a `.then` block. The `.then` block needs to return a Promise in order to be asynchronously caught, however the thrown error cannot be caught in the Promise body. So basically the answer is, you should not be throwing a synchronous exception in async code; javascript requires the error be passed to the callback and handled there OR for the function to be 'promisified' and reject on error. – serakfalcon Mar 19 '18 at 02:31
-1

From Chai documentation :

When no arguments are provided, .throw invokes the target function and asserts that an error is thrown.

So you could something like

expect(Matcher(1).case(1, () => {})).to.throw
Merlin
  • 224
  • 1
  • 11
-1

If your tested code calls setTimeout with a callback that throws and no-one is catching this is exception then:

1) this code is broken

2) the only way to see that problem is platform global exception handler like process.on('uncaughtException' mentioned by user ComFreek

The last resort chance is to stub setTimeout for duration of test (for example using sinon.stub) or just manually.

In such stubbed setTimeout you can decorate timeout handler, detect exception and call appropriate asserts.

NOTE, this is last resort solution - your app code is broken and should be fixed to properly propagate errors, not only for testing but ... well, to be good code.

Pseudocode example:

it('test', (done) => {

    const originalSetTimeout = setTimeout;
    setTimeout = (callback, timeout) => {
        originalSetTimeout(() => {
            try {
                callback();
            } catch(error) {
                 // CONGRATS, you've intercepted exception
                 // in _SOME_ setTimeout handler
            }
        }, timeout)
    }
    yourTestCodeThatTriggersErrorInSomeSetTimeoutCallback(done);
})

NOTE2: I intentionally didn't wrote proper async cleanup code, it's a homework. Again, see sinon.js and its sandbox

NOTE3: It will catch all setTimeout calls during test duration. Beware, there are dragons.

Zbigniew Zagórski
  • 1,921
  • 1
  • 13
  • 23
  • "your app code is broken and should be fixed to properly propagate errors, not only for testing but ... well, to be good code." No, it is not broken. Throwing an error is not part of the API, it's just optional way to highlight library user he messed up and forget to execute the chain (TypeScript has no way of statically detecting this). – menfon Mar 19 '18 at 05:53
  • The exception is NEVER meant to be cough. It would be dumb to introduce behemoth verbosity beyond point of usable, just to have a clean test. What is a point of a library, to serve library user (= concise, nice API to use) or to have beautiful easy clean tests? Please, do not generalize next time unless you know all the facts. – menfon Mar 19 '18 at 05:53
  • I might not understand question, but still i would argue that throwing exceptions that can be caught only by global exception handler is hardly a good practice. You write: "Throwing an error is not part of the API" - then you can't "cleanly" test stuff that is not part of API without hacks like hijacking globals. – Zbigniew Zagórski Mar 19 '18 at 10:33
  • 'You write: "Throwing an error is not part of the API" - then you can't "cleanly" test stuff that is not part of API' Why? Why can't one cleanly test if exception was thrown, regardless of its origin (Mocha internally does this)? This seems to me like a limitation of JS and/or test framework. This exception is only a debug tool for library users, not all will use it, and will not use it in production. This debug option is used as a workaround because of impossibility of checking fluent api chain end in JS without mangling API - https://stackoverflow.com/questions/28361837/js-method-chain-tail. – menfon Mar 19 '18 at 13:02