realm-js icon indicating copy to clipboard operation
realm-js copied to clipboard

Cannot use both mocked realm and actual realm in a test

Open Yupeng-li opened this issue 1 year ago • 5 comments

How frequently does the bug occur?

Always

Description

If I mock realm in a test file, and then import the real realm in a particular test, it will give this error:

{{ TypeError: Cannot redefine property: Realm}}

The error is from this line. Object.defineProperty was called twice.

To reproduce the issue, see example below.

jest.mock('realm')

it('a test', () => {
    const Realm = jest.requireActual('realm')
    // ...
})

Stacktrace & log output

No response

Can you reproduce the bug?

Always

Reproduction Steps

No response

Version

12.1.0

What services are you using?

Local Database only

Are you using encryption?

No

Platform OS and version(s)

Mac OS 13.5.1 (22G90)

Build environment

Which debugger for React Native: ..

Cocoapods version

1.12.1

Yupeng-li avatar Sep 08 '23 12:09 Yupeng-li

@Yupeng-li Mocking Realm in Jest isn't trivial. You might want to seek inspiration in https://github.com/realm/realm-js/issues/370#issuecomment-1464892940

kneth avatar Sep 08 '23 13:09 kneth

Hi @kneth , thank you! My issue isn't about how to mock realm, but realm module throws when it's imported multiple times. The sample code above shows the scenario when it could be imported twice. Our use case is slightly different. We use a jest custom matcher which imports Realm. Our tests also import realm, so the module is imported twice.

I temporarily fixed the issue I reported but then I got errors like:
Error: Expected value to be an instance of List, got an instance of List Error: Illegal constructor: Results objects are read from managed objects only.

It seems that assertion is checking instance type against two copies of the module. We didn't have the issue with Realm v11, so I am wondering if this is something you could fix?

Best regards

Yupeng-li avatar Sep 08 '23 14:09 Yupeng-li

There is a more common scenario. If I mock a file that imports Realm, the TypeError will be thrown. Please let me know if you need a min repo.

// functionB.ts

import Realm from 'realm' 

export const functionB = () => {
    console.log('function B', typeof Realm)
}
// some.test.ts
import Realm from 'realm' // this is required to reproduce the issue 
import { functionB } from './functionB'

jest.mock('./functionB') // This crashes the test because of the error: TypeError: Cannot redefine property: Realm

it('a test', () => {
    console.log('ss', typeof Realm) // This could be the code pre-populating data to realm, e.g. realm.create(...)
    functionB()
})

Yupeng-li avatar Sep 08 '23 15:09 Yupeng-li

I have the same issue, the test stopped working since upgrading to realm v12.3.0

a396901990 avatar Dec 05 '23 17:12 a396901990

Hi @kraenhansen I'm going through a bunch of upgrades on our apps atm (RN, 3rd-party deps, etc) and I was just wondering if there was any news on this issue yet? It's our remaining blocker to upgrade from Realm 11 to 12.

liamjones avatar May 14 '24 12:05 liamjones

Hi @kraenhansen, you mentioned before that you thought this might be a quick thing to fix.

With Realm going EOL and you working on other projects at Mongo I understand you might never get a chance to look at fixing this now.

Even if you can't fix this yourself, can you point us in the right direction in the codebase so we can look at patching this ourselves?

liamjones avatar Nov 28 '24 10:11 liamjones

Looking into this, I've actually already done the change (it was included in this huge refactor) and if it was what I thought it was, it should be fixed in v12.6.1.

Notice how the Realm property is defined as configurable: false in the old code: https://github.com/realm/realm-js/blob/966e44777350d4174c84f8787727dacd6b9759a1/packages/realm/src/Realm.ts#L1847-L1868

The code was moved and now it's configurable: true: https://github.com/realm/realm-js/blob/7e0120aa9ba4aa6ef5e9f065d709e3661b1f9e0a/packages/realm/src/deprecated-global.ts#L45

Can you verify that this has been resolved and if not, provide an updated stacktrace?

I'm sorry I didn't realize that we could close this issue based on that refactoring PR - a lot of things went into that 😬

kraenhansen avatar Nov 28 '24 13:11 kraenhansen

Looking into this, I've actually already done the change (it was included in https://github.com/realm/realm-js/pull/6492) and if it was what I thought it was, it should be fixed in v12.6.1.

Oh, that's a pleasant surprise!

Can you verify that this has been resolved and if not, provide an updated stacktrace?

Sure, this was blocking us moving from realm 11 to 12 previously so I'll have to resurrect the 11->12 changes we needed to make but I'll let you know. Many thanks!

liamjones avatar Nov 28 '24 13:11 liamjones

There is this example for realm mock in jest, its working for me as far as possible. Maybe can help someone

https://gist.github.com/hyb175/beb9ceed4c34300ba7c77d3d6d44ae52

vinibgoulart avatar Nov 28 '24 13:11 vinibgoulart

Hi again @kraenhansen,

I'm trying out 12.14.0 and you're right, the "Cannot redefine property: Realm" problem is gone. 🥳

But the other issues Yupeng mentioned remain. 😢

I've pulled together a minimal repro showing off one of them (hoping that both have the same cause), would you be able to take a look?

  1. Clone https://github.com/liamjones/RealmMockIssue
  2. npm install
  3. npm run test
  4. Test will fail with Illegal constructor: Results objects are read from managed objects only. and Jest doesn't exit
  5. Comment out the mocking of the file that has an import from Realm inside it: https://github.com/liamjones/RealmMockIssue/blob/main/tests/realm.test.ts#L6
  6. npm run test
  7. Test passes but Jest still doesn't exit

liamjones avatar Dec 03 '24 14:12 liamjones

Erhm. I've spent quite a bit of time trying to understand what's going on there - unfortunately without luck this far.

Jest's automock feature seems very powerful (and intrusive). The meat of the issue is, that Jest (somehow - unbeknownst to me) manages to break an internal invariant of the SDK.

For some reason, the binding.Results bound as constructor for the instance returned by binding.Results.fromTable here:

https://github.com/realm/realm-js/blob/05deac9b58143f654698d06f8766a840ae9af2c8/packages/realm/src/Realm.ts#L967

is not the same as binding.Results within Results.ts:

https://github.com/realm/realm-js/blob/05deac9b58143f654698d06f8766a840ae9af2c8/packages/realm/src/Results.ts#L61

internal.constructor.name === binding.Results.name // true
internal.constructor === binding.Results // false
internal instanceof binding.Results // false

I have no clue as to why this happens and I don't understand why Jest even touch the binding.Results or binding.Results.fromTable in the first place - is the jest.mock('../src/hasRatedThisVersion.ts') call transitive somehow?

kraenhansen avatar Dec 03 '24 22:12 kraenhansen

Thanks for taking a look @kraenhansen.

The fact that it's the automock process breaking this is interesting.

As far as I understand it, automocking does 'execute' the module being mocked to work out its exports shape. I would have thought it'd only have an impact on the directly mocked module, not any of the transitive dependencies but apparently not...

At a guess, I suspect it might be down to this code where Jest transforms the hasRatedThisVersion code, and uses the Node vm module to create a context and run the module script within it. My guess is that the Results object created in the alternate context is somehow leaking out of the mock-making process for the hasRatedThisVersion module and then the instanceof doesn't match up later.

But this is all guess work, maybe I need to raise an issue with the Jest project, they might have a better idea of what's going on.

The fact that it's the automock code specifically causing the issue does give us a workaround though. If we manually mock by providing a factory function:

jest.mock('../src/hasRatedThisVersion.ts', () => { hasRatedThisVersion: jest.fn() });

Then the automocking code is skipped and things work fine. It's slightly annoying having to maintain a manual copy of the module's exports but it's not the end of the world if we can't do anything more.

liamjones avatar Dec 04 '24 10:12 liamjones

It's slightly annoying having to maintain a manual copy of the module's exports but it's not the end of the world if we can't do anything more.

For what it's worth, I noticed jest.mock takes a type parameter and although a bit verbose, you can leverage that to gain type-safety:

jest.mock<typeof import('../src/hasRatedThisVersion.ts')>('../src/hasRatedThisVersion.ts', () => {
  return { hasRatedThisVersion: jest.fn<() => boolean>() };
});

kraenhansen avatar Dec 05 '24 09:12 kraenhansen