Skip to content

Commit

Permalink
fix: stop changing the behaviour of spies on mockFn.mockReset (#13692)
Browse files Browse the repository at this point in the history
  • Loading branch information
feliperli committed Jan 6, 2023
1 parent 2fb6f68 commit eace3a1
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@

- `[jest-environment-node]` fix non-configurable globals ([#13687](https://github.com/facebook/jest/pull/13687))
- `[@jest/expect-utils]` `toMatchObject` should handle `Symbol` properties ([#13639](https://github.com/facebook/jest/pull/13639))
- `[jest-mock]` fix mockReset and resetAllMocks undefined return ([#13692](https://github.com/facebook/jest/pull/13692))
- `[jest-resolve]` Add global paths to `require.resolve.paths` ([#13633](https://github.com/facebook/jest/pull/13633))
- `[jest-runtime]` Support Wasm files that import JS resources ([#13608](https://github.com/facebook/jest/pull/13608))
- `[jest-snapshot]` Make sure to import `babel` outside of the sandbox ([#13694](https://github.com/facebook/jest/pull/13694))
Expand Down
2 changes: 1 addition & 1 deletion docs/MockFunctionAPI.md
Expand Up @@ -126,7 +126,7 @@ The [`clearMocks`](configuration#clearmocks-boolean) configuration option is ava

Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also removes any mocked return values or implementations.

This is useful when you want to completely reset a _mock_ back to its initial state. (Note that resetting a _spy_ will result in a function with no return value).
This is useful when you want to completely reset a _mock_ back to its initial state.

The [`resetMocks`](configuration#resetmocks-boolean) configuration option is available to reset mocks automatically before each test.

Expand Down
24 changes: 24 additions & 0 deletions packages/jest-mock/src/__tests__/index.test.ts
Expand Up @@ -1215,6 +1215,30 @@ describe('moduleMocker', () => {
expect(fn.getMockName()).toBe('jest.fn()');
});

test('after mock reset, the object should return to its original value', () => {
const myObject = {bar: () => 'bar'};

const barStub = moduleMocker.spyOn(myObject, 'bar');

barStub.mockReturnValue('POTATO!');
expect(myObject.bar()).toBe('POTATO!');
barStub.mockReset();

expect(myObject.bar()).toBe('bar');
});

test('after resetAllMocks, the object should return to its original value', () => {
const myObject = {bar: () => 'bar'};

const barStub = moduleMocker.spyOn(myObject, 'bar');

barStub.mockReturnValue('POTATO!');
expect(myObject.bar()).toBe('POTATO!');
moduleMocker.resetAllMocks();

expect(myObject.bar()).toBe('bar');
});

test('mockName gets reset by mockRestore', () => {
const fn = jest.fn();
expect(fn.getMockName()).toBe('jest.fn()');
Expand Down
12 changes: 10 additions & 2 deletions packages/jest-mock/src/index.ts
Expand Up @@ -509,6 +509,7 @@ export class ModuleMocker {
private _mockConfigRegistry: WeakMap<Function, MockFunctionConfig>;
private _spyState: Set<() => void>;
private _invocationCallCounter: number;
private _originalFn: WeakMap<Mock, Function>;

/**
* @see README.md
Expand All @@ -521,6 +522,7 @@ export class ModuleMocker {
this._mockConfigRegistry = new WeakMap();
this._spyState = new Set();
this._invocationCallCounter = 1;
this._originalFn = new WeakMap();
}

private _getSlots(object?: Record<string, any>): Array<string> {
Expand Down Expand Up @@ -775,7 +777,12 @@ export class ModuleMocker {

f.mockReset = () => {
f.mockClear();
this._mockConfigRegistry.delete(f);
const originalFn = this._originalFn.get(f);
const originalMockImpl = {
...this._defaultMockConfig(),
mockImpl: originalFn,
};
this._mockConfigRegistry.set(f, originalMockImpl);
return f;
};

Expand Down Expand Up @@ -1215,7 +1222,7 @@ export class ModuleMocker {
return original.apply(this, arguments);
});
}

this._originalFn.set(object[methodKey] as Mock, original);
return object[methodKey] as Mock;
}

Expand Down Expand Up @@ -1401,6 +1408,7 @@ export class ModuleMocker {
}

resetAllMocks(): void {
this._spyState.forEach(reset => reset());
this._mockConfigRegistry = new WeakMap();
this._mockState = new WeakMap();
}
Expand Down

3 comments on commit eace3a1

@robhogan
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimenB (commenting here because the issue is closed) - Just FYI, this probably should've been labelled breaking as well. This is the kind of thing it broke for us, which worked in 29.2 and doesn't seem egregiously "wrong"

jest.spyOn(global.console, 'log');

describe('dog', () => {
  beforeEach(() => {
    jest.resetAllMocks();
  });

  it('barks to console', () => {
    dog.bark();
    expect(console.log).toHaveBeenCalledWith('woof'); // Error: console.log is not a mock
  });
  
  it('does something else with mocks that needs resetting', () => {
    // ...messing with mocks
  });
});
// ...

@SimenB
Copy link
Member

@SimenB SimenB commented on eace3a1 Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you open a new issue?

I'm kinda surprised reset removes the mock function entirely - that seems like a bug to me? From my reading of the docs, it should essentially be that same as doing jest.spyOn(global.console, 'log'); again

@mrazauskas
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be a bug which got fixed later in #13866?

Please sign in to comment.