jest

jest copied to clipboard

Summary

Setting HEAD to commit bf4d2d6013eebf0b02b9ec3cac57c3641bb02071 and running the tests will show that mocking of getters/setters on automatically mocked classes did not work at all. This request fixes that problem whilst maintaining existing functionality. All (non-mercurial) tests currently pass.

This is my first pull request so I do not know if the request number is the same as the issue number and have thus not updated the Changelog.md file with this fix. Apologies for any inconvenience.

Fixes #13140

Test plan

Test coverage can be seen in the aforementioned commit.

related issue https://github.com/facebook/jest/issues/13140

Thanks for sending a PR @staplespeter! There are conflicts - would you be able to resolve them? I assume the conflicts come from #13247

Hi @SimenB, yes I'll update the pull request.

@SimenB Pull request updated. I've made some assumptions about the type changes in #13247 and dynamically typecast Mocked<T> to PropertyDescriptor to get things to work. TBH this is from a position of mostly ignorance, however the existing tests for my changes still pass.

Any questions do please ask of course.

I've made some assumptions about the type changes in #13247 and dynamically typecast Mocked to PropertyDescriptor to get things to work.

Can spot this typecast ;D Could you point me to the line, please?

Regarding the type changes from #13247. All should be good if running yarn build and yarn test-types pass without errors. Note that type tests run agains the build, so remember to rebuild each time you make changes.

I've made some assumptions about the type changes in #13247 and dynamically typecast Mocked to PropertyDescriptor to get things to work.

Can spot this typecast ;D Could you point me to the line, please?

Regarding the type changes from #13247. All should be good if running yarn build and yarn test-types pass without errors. Note that type tests run agains the build, so remember to rebuild each time you make changes.

Apologies, I got dragged out the door for dinner and hadn't made the commit... The changes should be apparent now, in the latest commit.

I have run yarn build, yarn jest and yarn test-types, with passes. Mercurial tests were skipped.

Checked out the branch and looked through your change. I think all is fine with types. Looks right for my eye.

I have updated the code comments as requested. The removal of the "object._esModule" check in getSlots() did not effect any test results. This case is handled later when assigning the metadata in "getMetadata()"

https://github.com/facebook/jest/releases/tag/v29.1.0

@staplespeter I've had to revert this as it broke mocking in a work project.

import { Datastore } from '@google-cloud/datastore';

jest.mock('@google-cloud/datastore');

So it seems we're missing a test 🙂 Could you revisit?

@SimenB Yes of course, I'll take a look ASAP

Great, thanks! No immediate rush 👍

@SimenB can you tell me how to recreate the problem you encountered? I didn't receive any exceptions when executing jest.mock('@google-cloud/datastore');. Apologies if i have missed something obvious

Just need to instantiate it

import { Datastore } from '@google-cloud/datastore';

jest.mock('@google-cloud/datastore');

new Datastore();

@SimenB I have added a fix and a test, ran the local build and tests, and created a new pull request - https://github.com/facebook/jest/pull/13391

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.