FluffySpoon.JavaScript.Testing.Faking icon indicating copy to clipboard operation
FluffySpoon.JavaScript.Testing.Faking copied to clipboard

Default value (object of type Proxy) can give false positives when testing validations

Open SergiiVolodkoWorking opened this issue 4 years ago • 4 comments

Hi, Thanks for an amazing lib! It definitely makes work with JavaScript more human-friendly! :) I've bumped into a small difference with NSubstitute behaviour and wondering if maybe I just can't find a proper syntax to turn on or off a setting to make things work similarly to what I used to in C#:)

So what I'm trying to achieve. I have an interface

interface Worker {
   SUPPORTED_WORK: number[];
   doSomething();
}

And this kind of usage of the interface in some service:

constructor (private worker: Worker) {}
....

public someServiceMethod(workId) {
   if (!this.worker.SUPPORTED_WORK.includes(workId)) {
      throw Error("Invalid work");
   }
...

Currently, when I setup a test to cover this validation, if I don't specifying any behaviour for SUPPORTED_WORK property, the test automatically passes.

const worker = Substitute.for<Worker>();
const service = new MyService(worker);

service.someServiceMethod(42);
...//some test logic

Basing on the NSubstitute behaviour, I would expect this usage to throw an error, since the SUPPORTED_WORK property is missing or .include behaviour returns null or something like that by default.

At the moment, if the mock is unset .include(workId) returns a Proxy object, which is truthy. It makes the if-statement pass and giving a false positive in my test. Even if I wrap this check into some method like:

   if (!this.worker.isWorkSupported(workId)) {
      throw Error("Invalid work");
   }

The issue remains.

Wondering, if I can just tweak some setting to prevent false positives in that kind of checks when I miss to setup returns of my mock?

SergiiVolodkoWorking avatar Jan 13 '21 20:01 SergiiVolodkoWorking

Hi @SergiiVolodkoWorking Thanks for the detailed issue report! Due to javascript limitations, you need to setup the mock to return false (to make your example work). If we threw everytime a function or property of a mock is called, we wouldn't be able to record that execution -> no assertions possible (received, didNotReceive, ...). And we need to return a proxy in order to handle the mock functions:

worker.isWorkSupported(Arg.any()).returns(true) // if worker.isWorkSupported() returned null, a TypeError would be thrown

If you had any ideas how this could change / improve, we are happy to hear suggestions and feedback

notanengineercom avatar Jan 15 '21 22:01 notanengineercom

The only workaround I can think of, is implementing a custom Symbol.toPrimitive function. That should cast the proxy into 0, '' or null(or undefined?). What do you think @ffMathy ?

notanengineercom avatar Sep 12 '21 21:09 notanengineercom

Hmm not sure what you mean here. Can you elaborate? 😅🙏

ffMathy avatar Sep 13 '21 05:09 ffMathy

Hmm I think it was late last night and I was sleepy 😪 I somehow missed that .toPrimitive only works for some of the unary operators

notanengineercom avatar Sep 13 '21 17:09 notanengineercom