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

fix(mock): allow to mock methods in getters #10156

Merged
merged 1 commit into from Oct 19, 2020

Conversation

@SomaticIT
Copy link
Contributor

@SomaticIT SomaticIT commented Jun 10, 2020

Summary

In TypeScript 3.9.x, module "re-export" is done using readonly accessor.
It breaks all tests written using jest.spyOn mocks.
A solution is to automatically mock methods that are wrapped in getters.

Test plan

/**********************
 * module-a
 **********************/

// file: lib/feature.ts
export function myFunction(): boolean {
    return true;
}

// file: index.ts
export * from "./lib/feature.ts"

/**********************
 * submodule
 **********************/

// file: lib/subfeature.ts
import * as mod from "module-a";

export function mySubFunction(): boolean {
    return mod.myFunction();
}

// file: lib/subfeature.spec.ts
import * as submod from "./subfeature";
import * as mod from "module-a";

describe("mySubFunction", () => {
    let spy: jest.SpyInstance;
    beforeEach(() => { spy = jest.spyOn(mod, "myFunction"); });
    afterEach(() => { spy.mockRestore(); });

    it("should call mod.myFunction()", () => {
        submod.mySubFunction();
        expect(spy).toHaveBeenCalled();
    });
});

If module-a was built using TypeScript < 3.9, this will work.
If module-a was built using TypeScript >= 3.9, this will throw with error TypeError: Cannot set property query of #<Object> which has only a getter.

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Jun 10, 2020

Hi @SomaticIT!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Jun 10, 2020

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@SimenB
Copy link
Collaborator

@SimenB SimenB commented Jun 23, 2020

Don't we support this already via the third option?

https://jestjs.io/docs/en/jest-object#jestspyonobject-methodname-accesstype

@SimenB
Copy link
Collaborator

@SimenB SimenB commented Oct 19, 2020

@SomaticIT ping 🙂

@SomaticIT
Copy link
Contributor Author

@SomaticIT SomaticIT commented Oct 19, 2020

@SimenB

The third option, allows to replace the get accessor function of a property.

In my case, we want to change the function which is returned by the get accessor function.
It's an edge case which happens when compiling the code using TypeScript >= 3.9.

So the difference:
object > property > get > function

  • accessType option: target the get
  • my fix: target the function

This fix allows to easily handle this case with no overhead for the user.

@SimenB SimenB requested review from jeysal and thymikee Oct 19, 2020
@SimenB
Copy link
Collaborator

@SimenB SimenB commented Oct 19, 2020

Aha! Yeah, I think this makes sense, then. Mind giving this a quick rebase (or merge in master)?

@SomaticIT SomaticIT force-pushed the SomaticIT:@fix/spyon-method-getter branch 2 times, most recently from 59d3811 to 06e596c Oct 19, 2020
@SomaticIT SomaticIT force-pushed the SomaticIT:@fix/spyon-method-getter branch from 06e596c to 0531a25 Oct 19, 2020
@SomaticIT
Copy link
Contributor Author

@SomaticIT SomaticIT commented Oct 19, 2020

@SimenB
Rebase done!

@jeysal
jeysal approved these changes Oct 19, 2020
@SimenB SimenB merged commit 30e8020 into facebook:master Oct 19, 2020
22 checks passed
22 checks passed
cleanup-runs
Details
Running TypeScript compiler & ESLint
Details
Node v10.x on ubuntu-latest
Details
Node v10.x on macOS-latest
Details
Node v10.x on windows-latest
Details
Node v12.x on ubuntu-latest
Details
Node v12.x on macOS-latest
Details
Node v12.x on windows-latest
Details
Node v13.x on ubuntu-latest
Details
Node v13.x on macOS-latest
Details
Node v13.x on windows-latest
Details
Node v14.x on ubuntu-latest
Details
Node v14.x on macOS-latest
Details
Node v14.x on windows-latest
Details
Facebook CLA Check Contributor License Agreement is valid!
Details
ci/circleci: test-jest-circus Your tests passed on CircleCI!
Details
ci/circleci: test-node-10 Your tests passed on CircleCI!
Details
ci/circleci: test-node-12 Your tests passed on CircleCI!
Details
ci/circleci: test-node-13 Your tests passed on CircleCI!
Details
ci/circleci: test-node-14 Your tests passed on CircleCI!
Details
ci/circleci: test-or-deploy-website Your tests passed on CircleCI!
Details
facebook.jest #20201019.24 succeeded
Details
@SimenB
Copy link
Collaborator

@SimenB SimenB commented Oct 19, 2020

Thanks!

@SomaticIT SomaticIT deleted the SomaticIT:@fix/spyon-method-getter branch Oct 20, 2020
This was referenced Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.