spectator

spectator copied to clipboard

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[X] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

Currently, when we use mockProvider and want to set a default function value, we need to explicitly declare the property as a function. Even further, the function created in the mock doesn't become a mock, it's actually a raw function. So given an AuthService like this:

class AuthService {
  isAdmin: boolean;

  async login(path): Promise<boolean> {
    // ... 
  }
}

I need to create a mock like this if I want some default values:

const authServiceMock = mockProvider(AuthService, {
  isAdmin: false,
  login: jest.fn(() => Promise.resolve(null)),
});

Which isn't really bad, but can be repetitive if we have many methods to mock and given how jasmine's createSpyObject automatically creates a spy with the return value, I actually expected there was a similar feature already.

Expected behavior

I can see two possible options for implementing this feature:

1. since the first argument is the original class, we could check which properties in the second argument are functions in the prototype and assign them as jasmine.createSpy() or jest.fn when creating the mock.

With this, the following mock would create an instance where isAdmin is a property with value true and login is a spy which returns a resolved promise of null.

const authServiceMock = mockProvider(AuthService, {
  isAdmin: false,
 // typeof AuthService.prototype.login === 'function', so it could be inferred as a spy here
 // Slightly cleaner syntax and avoids repetition for classes with many methods
  login: Promise.resolve(null),
});

2. accept one extra parameter and separate properties from methods;

const authServiceMock = mockProvider(AuthService, {
  isAdmin: false,
}, {
  // One extra argument for auto spies.
  // Methods before properties would keep jasmine's createSpyObj order, but this would avoid breaking changes.
  login: Promise.resolve(null),
});

What is the motivation / use case for changing the behavior?

1. compatibility with jasmine's createSpyObj functionality;
2. make setting default return values and spies more straight forward and less repetitive;

Are you saying that you wish to skip this part?

authServiceMock.login.and.callFake(() => fake);

Yes, that's basically it. To provide a more practical use case:

Where I work at, we use ngx-translate, which ships with a TranslateService that has a method called instant and is used in multiple components. When writing components tests, it doesn't really matter the return of this service, but if it keeps returning undefined, it may break a lot of tests, so I have a file called translate-service.provider.ts where I leave a mock that at least returns the same type what my component is expecting (a string). I built my own createSpyObj function since I'm using jest, so the file currently looks like this:

import { mockProvider } from '@ngneat/spectator/jest';
import { TranslateService } from '@ngx-translate/core';
import { of } from 'rxjs';

import { createSpyObj } from '@app/core/helpers/tests/create-spy-object';

export const translateServiceMockProvider = mockProvider(TranslateService, createSpyObj({
  instant: 'translated',
  get: of(''),
  getParsedResult: of(''),
}));

Then in my unit tests I can use:

const createComponent = createComponentFactory({
  component: MyComponent,
  providers: [
    translateServiceMockProvider,
  ],
});

It's much straight forward than either setting the spies in every single unit test and a bit less repetitive than this:

export const translateServiceMockProvider = mockProvider(TranslateService, {
  instant: jest.fn(() => 'translated'),
  get: jest.fn(() => of('')),
  getParsedResult: jest.fn(() => of('')),
});

These default return values are useful when you know you'll be almost always accessing some property from the method's return (e.g arrays, object destructuring and so on).

It's not making much difference for me particularly since I created my own createSpyObj function, but I think it would be nice for everobody if this was a default behavior. It also took me a while to realize that the createSpyObject function from spectator only supports property values, so it might be someon else's issue too. Also, it's one more function I could get rid of.

Edit: also, I'm using restoreMocks: true in my jest config. I think that if I remove it, the factory will reuse the mocks across all tests since I define them as an argument. I'll check this later and update here if you find this feature request relevant and possible.

There's no problem with me seeing a PR for this feature. And btw, I recommend using ngneat/transloco :)

And btw, I recommend using ngneat/transloco :)

I thought you'd say that hahaha. Unfortunatetly, there's a lot of things done in the project already before I joined the company. But maybe I'll convince them to try a migration the same way I'm trying to convince them to use until-destroy.

About the PR, I'll start work on it. Do you prefer to infer methods according to the class prototype or simply accept an extra paremeter for explicitly defining methods?

The latter.