Skip to content

Fix for .returns object inequality - #79

Merged
ffMathy merged 5 commits into
ffMathy:masterfrom
notanengineercom:fix-primitive-value
Feb 14, 2020
Merged

Fix for .returns object inequality#79
ffMathy merged 5 commits into
ffMathy:masterfrom
notanengineercom:fix-primitive-value

Conversation

@notanengineercom

Copy link
Copy Markdown
Collaborator

Fixes #36

This fixes the issue of ticket 36. It didn't have anything to do with promises, but with how the arguments equality for a mock with .return was performed.
So if we mocked a method where one of the arguments was an object or class, unless the object had the same reference, areArgumentsEqual function returned false.

const a = { value: 1 }
const b = a
a === b // true

const c = { value: 1 }
a === c // false

When this ocurred, the mocked method / property, instead of returning the expected value, it returned context.proxy, which had all sort of side effects, one of them being that sometimes an error was thrown with the message Cannot convert object to primitive value.


I made a little change on the proxies, they are based now on a dummy SubstituteJS class. I'm not sure yet if this will help with debugging when something goes wrong, but by doing that, and adding the getOwnPropertyDescriptor handler, instead of getting the arguments not matching error with the generic [[Function]], it will write [[SubstituteJS]]

Error {
    message: `Expected 1 call to the method storeUpdates with arguments.....
    ....All calls received to method storeUpdates:␊
    -> call with arguments [[Function]]`,
  }
// ------
VS
// ------
Error {
    message: `Expected 1 call to the method storeUpdates with arguments....
    .....All calls received to method storeUpdates:␊
    -> call with arguments [[SubstituteJS]]`,
  }

When a property / method is mocked with returns, and one of the arguments is an object (or a class or a function), the `areArgumentsEqual` function in utilities failed to verify. It executed the default behaviour `a === b`, which is never true for objects (only if a and b have the same object reference)
@ffMathy

ffMathy commented Feb 14, 2020

Copy link
Copy Markdown
Owner

This was by design, and is also how NSubstitute (which this library is a port of) behaves.

If you want to match arguments by value, can't you use Arg.is instead?

@notanengineercom

Copy link
Copy Markdown
Collaborator Author

I think I wasn't clear enough, the issue appears when evaluating the argument of a mock defined with .returns, not with .received

// In the unit test
const db = Substitute.for<IFetch>()
const data = IData.create()
db.getUpdates(Key.create()).returns(Promise.resolve(data))
const service = new Service(db)
await service.handle()

// In the Service class
.
.
.
 const updateData = await this.getData(Key.create()) // returns context.proxy instead of data

When the class or function on the unit test calls db.getUpdates(Key.create()), I would expect to receive a Promise that resolves with data.

The substitute.received method works as expected, and there is no issue with Arg.is.

@ffMathy

ffMathy commented Feb 14, 2020

Copy link
Copy Markdown
Owner

Ah I see - my bad! Nice fix!

@ffMathy
ffMathy merged commit af5ca27 into ffMathy:master Feb 14, 2020
@notanengineercom
notanengineercom deleted the fix-primitive-value branch February 14, 2020 11:04
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

Successfully merging this pull request may close these issues.

Cannot convert object to primitive value

2 participants