Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
SergiiVolodkoWorking opened this issue Jan 13, 2021 · 4 comments
Milestone

Comments

@SergiiVolodkoWorking
Copy link

SergiiVolodkoWorking commented Jan 13, 2021

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Owner

ffMathy commented Sep 13, 2021

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

@notanengineercom
Copy link
Collaborator

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

@ffMathy ffMathy added this to the v2.0 milestone Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants