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

Suggestion: implementing auto-mocking capabilities #335

Closed
omermorad opened this issue Nov 9, 2021 · 18 comments
Closed

Suggestion: implementing auto-mocking capabilities #335

omermorad opened this issue Nov 9, 2021 · 18 comments

Comments

@omermorad
Copy link
Contributor

Hi,

The testing package in golevelup is great, the easy functionality of createMock makes it easy to mock things without any effort; @jmcdo29 - saw your issue at nest repo (nestjs/nest#6277 - feat(testing): add auto-mocking capabilities) and thought maybe I can implement it, so I did.

My questions are (1) Do you still want this feature? (2) Is this the right repo to create a PR in?

Here is a code snippet example:

describe('Unit Test', () => {
  let someService: DeepMocked<SomeService>;

  beforeAll(() => {
    someService = Unit.createTestingService(SomeService)
      .mock(ClientService)
      .with({
        foo: () => Promise.resolve({ token: '' }),
      })
      .mock(PayService)
      .with({
        bar: () => 'dogs are awesome',
      } as any)
      .compile();
  });

  describe('Test Something', () => {
    test('then do something', () => {
      expect(1).toBe(1);
    });
  });
});

Where all the rest of the dependencies of the class SomeService are auto-mocked, everything is using createMock() of course

@jmcdo29
Copy link
Contributor

jmcdo29 commented Nov 9, 2021

Oh! That's a cool way to go about it actually. There's an open PR in Nest's main repo to add this Auto mock kind of setup as well. I need to bring it back up and see what else we want to do on it

@omermorad
Copy link
Contributor Author

Thanks :))

Nice! Let me know if you wanna see the code, it's short.
Actually the more I think about it - it feels like it should be separated from nest's repo, it feels like it is something extra (exactly like createMock does) that needs its own package (and treat it as an extra)

The PR is open a long time for a while by the way :|

@WonderPanda
Copy link
Collaborator

@omermorad If you have a more general purpose NestJS wrapper for testing I'm definitely open to including it in this repository.

At the moment the mock functionality lives inside @golevelup/ts-jest which is just a general purpose mock that really doesn't have anything to do with NestJS. I don't think I'd want to update that package to take on any Nest related dependencies since the library is used by people from other ecosystems.

We could definitely consider adding a new package called for example @golevelup/nestjs-testing which depends on @golevelup/ts-jest and exposes your new Nest specific testing utilities.

@omermorad
Copy link
Contributor Author

omermorad commented Nov 10, 2021

@WonderPanda yes, you are totally right, the original package is for general purposes, so it doesn't make sense to add nest dependencies there.

Sounds good, so do you want me to create a fresh new PR that adds a new package called @golevelup/nestjs-testing? I'm almost finished with it

Another thing - what do you think about adding Mocha/Sinon support (maybe Sinon stub function) to the original @golevelup/ts-jest package, and then to update the package I'm about to create so they can both serve more different test runners?

@WonderPanda
Copy link
Collaborator

I'm definitely open to it assuming that we can get the type safety guarantees on feature parity with what is in ts-jest. We might need to rename that package too though to just be ts-testing or something if its going to support a bunch of different runners/frameworks beyond jest

@omermorad
Copy link
Contributor Author

But it's for nestjs specifically, I'm using the Reflector there

@WonderPanda
Copy link
Collaborator

Yeah, I understand. I think that we definitely want to have a dedicated package for the NestJS testing eg @golevelup/nestjs-testing which can handle anything to do with Reflector or any other NestJS dependencies.

In my previous comment I just meant that if you were looking at adding in functionality to the existing package @golevelup/ts-jest to support other frameworks eg mocha/sinon we should rename that package to be @golevelup/ts-testing since it will definitely extend past the current scope of the naming related to ts-jest.

I think it would make the most sense to split these initiatives across multiple PRs so that we can get the NestJS testing package in first and then we can figure out the best way to support mocha/sinon/etc

@omermorad
Copy link
Contributor Author

Ok, got you.
Sounds good, I will create two different PR's and we will discuss everything there(?) :)

@WonderPanda
Copy link
Collaborator

That sounds awesome, thanks @omermorad!

@omermorad
Copy link
Contributor Author

😎😎

@omermorad
Copy link
Contributor Author

omermorad commented Nov 15, 2021

@WonderPanda I want to draw your attention to something; I've made a little bit of a research (actually "us", thanks @potoos) and found this package called jest-mock-extended, can you take a short look? It seems that they took it a little step further, and I thought maybe to give it a try, but of course, to ask you first.

@potoos (a friend who works with me) - can you please explain the differences we discussed and how can it help us here with the new package we want to create?

@WonderPanda
Copy link
Collaborator

@omermorad It doesn't surprise me that someone has take the automocking further. The current implementation is a pretty small wrapper around Jest. If the other library is objectively better I'm fine to look at using it as the foundation for some NestJS testing utilities

@potoos
Copy link

potoos commented Nov 15, 2021

@WonderPanda the main differentiator between the libraries that we've found is the deep mocking capabilities for nested objects. We've found that @golevelup/ts-jest doesn't do this well out-of-the-box.

We couldn't quite understand why createMock didn't stub all of the first-level dependencies and only some. Could be our misuse.
We found that jest-mock-extended's mock did the trick.

@WonderPanda
Copy link
Collaborator

Are you able to show a quick snippet that demonstrates the difference between the two?

@omermorad
Copy link
Contributor Author

I can try :)

Example 1

Take a look on the query runner mock (queryRunnerMock), the mock function is imported from jest-mock-extended

describe('Some Unit Test', () => {
  let payoutService: PayoutService, service;

  const queryRunnerMock = mock<QueryRunner>();
  const errorMock = new Error('Some Error');

  beforeAll(() => {
    const { unit, unitRef } = Spec.createUnit<PayoutService>(PayoutService)
      .mock(PayoutDao)
      .using({
        createPayout: () => Promise.resolve({ id: 2 }),
      })
      .mock(ClientService)
      .using({
        getAccountId: () => Promise.resolve({ token: '' }),
      })
      .mock(Connection)
      .using({
        createQueryRunner: () => queryRunnerMock,
      })
      .compile();

    payoutService = unit;

    service = unitRef.get(SomeService);
  });

  describe('when something happens', () => {
    test('then check something', async () => {
      service.createTransfer.mockImplementationOnce(() => {
        throw errorMock;
      });

      payoutService.performActionForTest();

      expect(queryRunnerMock.startTransaction).toHaveBeenCalled();
      expect(queryRunnerMock.rollbackTransaction).toHaveBeenCalled();
      expect(queryRunnerMock.release).toHaveBeenCalled();
    });
  });
});

Example 2
In this case the Spec class is wrapping createMock function (I mean when we call .mock function in the builder), and not the function from jest-mock-extended (which is called deepMock). In this example we had to stub all the functions inside queryRunnerMock in order for it to work (when we tried to deep mock it using createMock it did not work).

The differences are hard to tell, I think it is related to the inner implementation of the Proxy (take a look at jest-mock-extended repo).

describe('PayoutService', () => {
  let payoutService: PayoutService, service;

  const queryRunnerMock = createMock<QueryRunner>({
    connect: jest.fn(),
    startTransaction: jest.fn(),
    commitTransaction: jest.fn(),
    rollbackTransaction: jest.fn(),
    release: jest.fn(),
  });

  const errorMock = new Error('Some Error');

  beforeAll(() => {
    const { unit, unitRef } = Spec.createUnit<PayoutService>(PayoutService)
      .mock(PayoutDao)
      .using({
        createPayout: () => Promise.resolve({ id: 2 }),
      })
      .mock(ClientService)
      .using({
        getAccountId: () => Promise.resolve({ token: '' }),
      })
      .mock(Connection)
      .using({
        createQueryRunner: () => queryRunnerMock,
      })
      .compile();

    payoutService = unit;

    service = unitRef.get(SomeService);
  });

  describe('when something happens', () => {
    test('then check something', async () => {
      service.createTransfer.mockImplementationOnce(() => {
        throw errorMock;
      });

      payoutService.performActionForTest();

      expect(queryRunnerMock.startTransaction).toHaveBeenCalled();
      expect(queryRunnerMock.rollbackTransaction).toHaveBeenCalled();
      expect(queryRunnerMock.release).toHaveBeenCalled();
    });
  });
});

I hope it makes sense for you

@omermorad
Copy link
Contributor Author

@WonderPanda any update?

@WonderPanda
Copy link
Collaborator

@omermorad I don't have a lot of time to investigate this further right now, currently trying to get a side project into market. If the functionality of jest-mock-extended works out better for what you're trying to accomplish I have no issues with using that as the foundation for a Nest testing package and am more than happy to help you incorporate it into @golevelup or if you'd prefer to publish it under your own org that's totally cool too

@underfisk
Copy link
Contributor

The auto mock will be available when #342 is released

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

No branches or pull requests

5 participants