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: respect prototypes when mocking #953

Merged
merged 4 commits into from May 20, 2024
Merged

fix: respect prototypes when mocking #953

merged 4 commits into from May 20, 2024

Conversation

npdev453
Copy link
Contributor

@npdev453 npdev453 commented Oct 9, 2023

If trying to mock class instance (object with prototype) nothing working.

  1. overrides totaly ignored (fixed by hasOwnProperty)
  2. result object miss his proto after "cloning"
class InjectableClass {
    fn1() {
        return 'a';
    }
    fn2() {
        return 'b';
    }
}

void t.test('Encoder', (t) => {
    const mockedInjectableInstance = t.createMock(new InjectableClass(), {
        fn1() {
            return '123457';
        },
    });
})

mockedInjectableInstance will:

 {}

Instead:

 {
    fn1: () => '123457';
    __proto__: InjectableClass
}

@isaacs
Copy link
Member

isaacs commented Oct 9, 2023

Hm, I can see how this would make t.createMock() more generally useful. I've been using it just to override imports, like:

import * as FS from 'node:fs'
await t.mockImport('../src/some-file.js', {
  fs: t.createMock(FS, {
    symlinkSync: mockSymlinkSync,
  }
})

But the name does imply that you can use it for mocking any random object, so that makes sense.

Needs some tests added to src/mock/test/index.ts to show the changed functionality. (Something that fails without the patch, passes with the patch, and brings coverage to 100%.)

src/mock/src/index.ts Outdated Show resolved Hide resolved
src/mock/src/index.ts Outdated Show resolved Hide resolved
@leonardoadame
Copy link

793c1c0

isaacs and others added 4 commits May 19, 2024 15:18
This also renames `globCwd` to the more appropriate `projectRoot`.

Verify that extension cycle detection doesn't result in false positives
(it was marking the relative path, not the fully resolved path, which
might be different if extended from a different place).

BREAKING CHANGE: any references to a coverage-map, before, or after in
an extension config will have to be updated to be relative to the file
containing the reference, which really is how it should have worked all
along.

Also, since this touched a ton of stuff anyway, formatted everything and
updated to prettier 3.
BREAKING CHANGE: plugins are now installed in a different location. This
should avoid issues with pnpm and workspaces, and in general make plugin
handling much cleaner.
@isaacs isaacs closed this in c80cbd8 May 20, 2024
@isaacs isaacs merged commit c80cbd8 into tapjs:main May 20, 2024
1 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants