Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Something is wrong with a cache #805

Open
theKashey opened this issue May 16, 2019 · 9 comments
Open

Something is wrong with a cache #805

theKashey opened this issue May 16, 2019 · 9 comments
Labels

Comments

@theKashey
Copy link

Derived from #798, #799 and theKashey/rewiremock#83

There is a simple test:

  • A.js <- reexports <- B.js <- reexports <- C.js
  • changing C.js, would change B and A results, which are the same.
// A.js
import B from './B';
export default B;

// B.js
import C from './C';
export default C;

// C.js
export default 42;

Test 1

 const subject = rewiremock.proxy(
      () => require('./A'),
      () => {
        rewiremock(() => require('./B')).with('bar')
      }
    );
    expect(subject.default).to.be.equal('bar')

In this test we are replacing B by "bar". It is working

Test 2

it('mock B', () => {
    const subject = rewiremock.proxy(
      () => require('./B'),
      () => {rewiremock(() => require('./C')).withDefault('baz')}
    );
    expect(subject.default).to.be.equal('baz')
  });

This time test would fail. C would be not mocked, as long as B cache would be reused, so it never gets a change to get mocked - execution enters ESM loader with "B", and exists with a result, never calls Module._load again.

Test 3

Add one more test, between these two

it('mock C', () => {
    const subject = rewiremock.proxy(
      () => require('./A'),
      () => {rewiremock(() => require('./C')).with('bar')}
    );
    expect(subject.default).to.be.equal('bar')
  });

It would fail, but test 2 which would be executed right after it, will pass.

Test 4

  • Keep only Test 2 - green
  • Duplicate it - green
  • Add Test3 - green
  • Add Test1 - the test after Test1 is red
  • replace A.js by module.exports = require('./B'); - all tests are green.

So - there is something like cache poisoning, which take a place after A.js

Add D.js

I've extended chain by D.js, then started modifying the code.

  • A->B(mock) - test ok, following fails
  • A->B->C(mock) - test ok, following fails
  • A->B->C->D(mock) - test ok, following ok. This is beyond my imagination.
  • B->C would crash following A->D.

I've also tried to run rewiremocks tests vs esm and majority failed due to cache related issues.

I've also tried to disable ESM cache, but it has no effect.

Theory

Some parts of rewiremock uses require.cache, while others uses Module._cache. While it's the same for nodejs and webpack, it might not be the case for esm.

@boneskull
Copy link

I think the cache-disabling flag just prevents it from being loaded from disk. Stuff still goes in an in-memory cache

@theKashey
Copy link
Author

Another tricky moment:

  • 'A' requires 'B' and 'C'.
  • 'C' requires 'B'
  • 'B' is mocked, thus not stored in the cache - call to the underlaying Module._load is not made.
  • 'B'-mock have a check - parent module should be 'A'. Ie it should mock it when required from 'A', but not from 'C'
  • it's working.
operation file parent parent of parent
require C.js A.js mock.spec.js
require B.js C.js A.js

Then I am adding esm

operation file parent parent of parent
require C.js A.js mock.spec.js
require B.js C.js mock.spec.js

From call to call the parent of C has changed. In nodejs a module could have only one parent, and 'B' was already "used" from "mock.spec", but that usage should not be cached, and any cache could mean nothing for Module._load level, as long as it exists before the cache line.

There is only one place which might set mock.spec as a parent of C.js - the test itself.
To be more concrete - the guided mock construction - rewiremock(() => require('./C')).with('bar').

How it works:

  • overloads Module._load
  • execute requires/imports
  • observes which files were required
  • set's mocks
  • remove overload.
  • should not have any effect on the module system and ever run any code.
  • by fact - these files are listed as children of mock.spec.js with loaded:false flag.

Probably it's now easy to create a first test for a problem - overloading Module._load with nothing should disable all side effects on the cache system.

The best solution - move cache border one level down. It's quite hard to debug esm, but Module._load is called from esm and calling a "real" Module._load would dive to esm again. Cache border right now is at first point, while should be at the second.

@boneskull
Copy link

The TL;DR of this issue is that “esm and rewiremock do not play nice together” to such a degree that the combination isn’t feasible beyond trivial use of rewiremock.

@boneskull
Copy link

I think jdalton is on an open-source vacation / starting new job, so don’t expect the usual prompt fix.

@theKashey
Copy link
Author

Look like scratchCache is the source of the problem.
While I've found a way to debug esm(provide a correct filename for Script in esm.js) - I could not understand how it is populated - there is no logic between require and rewiremock.

@jdalton
Copy link
Member

jdalton commented May 19, 2019

Hi @theKashey!

The scatchCache is used for the in-between phases of parsing and execution of ES modules. If you could create a small repro repo I can dig in and investigate a fix.

@jdalton jdalton added the bug label May 19, 2019
@theKashey
Copy link
Author

This test
https://github.com/theKashey/rewiremock/blob/master/_tests/mock.spec.js#L262-L274

👎 fail - node node_modules/mocha/bin/_mocha --require esm --ui bdd rewiremock/_tests/mock.spec.js --grep "^rewiremock called from a mock should mock only direct child due to callThrough mocked \/ async \: $"

👍 pass - node node_modules/mocha/bin/_mocha --compilers js:babel-core/register --ui bdd rewiremock/_tests/mock.spec.js --grep "^rewiremock called from a mock should mock only direct child due to callThrough mocked \/ async \: $"

I'll try to extract it in a shorter form, you will be able to integrate to esm's tests.

@jdalton
Copy link
Member

jdalton commented May 19, 2019

@theKashey

I'll try to extract it in a shorter form, you will be able to integrate to esm's tests.

Thank you!

@theKashey
Copy link
Author

https://github.com/theKashey/esm-bug-805

  • yarn test:babel - works
  • yarn test:esm - fails. It's a bit strange, but it has a reverse Module._load invocation.
    In the chain a->b->c->d, it would be a->d->c->b.

I've tried to make the example as complicated, as possible - but it just works, except this moment.

I've also update esm from 3.2.22 to 3.2.25 and look like THIS issue is resolved. At least the test I was playing with is green.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

3 participants