ffMathy / FluffySpoon.JavaScript.Testing.Faking

An NSubstitute port to TypeScript called substitute.js.
MIT License
203 stars 22 forks source link

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

Open SergiiVolodkoWorking opened 3 years ago

SergiiVolodkoWorking commented 3 years ago

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?

notanengineercom commented 3 years ago

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 commented 3 years ago

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 ?

ffMathy commented 3 years ago

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

notanengineercom commented 3 years ago

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