chai icon indicating copy to clipboard operation
chai copied to clipboard

expect().to.throw without invocation should warn

Open kellyselden opened this issue 4 years ago • 3 comments

Given a func that is not throwing. This code passes:

expect(() => func()).to.throw;

but this code does not:

expect(() => func()).to.throw();

This accident was hiding in our code. I think I've seen other chai assertions warn or error if used as a property and not a method. Apologies if this has been brought up before.

kellyselden avatar Apr 27 '20 17:04 kellyselden

The same is even trickier when you expect something not to throw.

For instance, the following pass unhindered:

expect(() => { throw Error(); }).not.to.throw;

maximeB avatar Feb 14 '24 10:02 maximeB

Good catch! Magical getters has been a problem since 2.x :disappointed:. Let's make sure this gets fixed. PRs welcomed!

keithamus avatar Feb 14 '24 11:02 keithamus

Hello, @keithamus I'm interested in this issue and looking at chai code.

The same phenomenon is currently occurring in the latest version, but it is difficult to resolve the error.

If we want to raise an error when you only look up a property without executing the assertion method, you will have to process the method's proxy to raise an error if the function is not called after a certain point after the lookup, but this does not seem to be a good idea. because delay makes one test case make one pass and one fail.

// this is sudo code
// Add proxy when adding properties.

export function addMethod(ctx, name, method) {
  var methodWrapper = (info) => function () {
    // Setting the `ssfi` flag to `methodWrapper` causes this function to be the
    // starting point for removing implementation frames from the stack trace of
    // a failed assertion.
    //
    // However, we only want to use this function as the starting point if the
    // `lockSsfi` flag isn't set.
    //
    // If the `lockSsfi` flag is set, then either this assertion has been
    // overwritten by another assertion, or this assertion is being invoked from
    // inside of another assertion. In the first case, the `ssfi` flag has
    // already been set by the overwriting assertion. In the second case, the
    // `ssfi` flag has already been set by the outer assertion.
    info.excute = true
    if (!flag(this, 'lockSsfi')) {
      flag(this, 'ssfi', methodWrapper);
    }

    var result = method.apply(this, arguments);
    if (result !== undefined)
      return result;

    var newAssertion = new Assertion();
    transferFlags(this, newAssertion);
    return newAssertion;
  };

  addLengthGuard(methodWrapper, name, false);

  Object.defineProperty(ctx, name,
    { get: function propertyGetter(...rest) {
      const info = {excute:false}
      setTimeout(()=>{
        console.log(info.excute)
        if(!excute) throw new Error("method is not excuted")
      })
      return proxify(methodWrapper(info), name)
      }
    , configurable: true
  });
}

A simple idea that comes to mind is to write an Eslint custom rule to warn you if the assertion method is not executed. How about solving the problem this way?

developer-bandi avatar Feb 18 '24 05:02 developer-bandi