Feature Request: Allow auto mocking of methods returning Observable with custom Subject

[DmitryEfimenko]

Describe the feature or problem you'd like to solve

I'd like to see if we can improve the Observable mocking story of ng-mocks.

The current solution of providing an override for a method/property where it would return a predefined observable is very limiting and I find myself never using this approach. Every time I tend to reach for the solution described in "Customizing observable streams" section. However, in order to minimize the amount of boilerplate, I came up with a function that automates the process. Here's the function (some types are not provided for the sake of brievety):

export function createObservableFlushTriggers<T, K extends keyof PickObservableMethods<T>>(
  classInstance: Spy<T> | jasmine.SpyObj<T>,
  ...methods: K[]
): FlushTriggersOf<T> {
  const flush = {} as FlushTriggersOf<T>;
  for (const method of methods) {
    const subject = new Subject();
    classInstance[method as any].and.returnValue(subject);
    flush[method as any] = {
      success: (val: any) => subject.next(val),
      error: (val: any) => subject.error(val)
    };
  }
  return flush;
}

Here's how it's used:

class ApiService {
  save(data) {
    return this.http.post(data);
  }
}

const deps = MockBuilder().mock(ApiService).build();

TestBed.configureTestingModule(deps);

// later in it statement:
// auto-spy is enabled so this gives me a SpyObj<ApiService>
const apiService = TestBed.inject(ApiService);

const flush = createObservableFlushTriggers(apiService, 'save');

// pushes the provided value through the subject
flush.save.success({ someData: true });

// pushes error to the subject
flush.save.error(new HttpErrorReponse({ ... }));

Proposed solution

I'm wondering if something like above can be built into ng-mocks.
In the best case scenario it would work without any configuration if auto-spy was turned on. Here's how I imagine it:

const deps = MockBuilder().mock(ApiService).build();

TestBed.configureTestingModule(deps);

const apiService = TestBed.inject(ApiService);

// ng-mocks adds a `flushSuccess` and `flushError` methods on all class methods that return Observable<any>
apiService.save.flushSuccess(data)

Please let me know what you think of the feasibility of the idea.

Additional context

It looks like it's tricky to identify at run-time the return type of the method. So that's where it might get tricky to make this work without any configuration.
If I'm not mistaken, TypeScript does not quite support this out of the box. However, some libraries out there can possibly do it

[satanTime]

Thank you for the suggestion. The main problem here is that apiService.save doesn't know anything about flushSuccess.

I think if we play with a helper function, it should work.

ngMocks.stub$(apiService, 'save').next(data);
ngMocks.stub$(apiService, 'save').error(data);

However, it requires to fetch apiService first and it might be too late if someone's decided to subscribe in a constructor.

Considering your example, why do you not want to use MockInstance?

beforeEach(() => MockBuilder().mock(ApiService));

it('test', () => {
  const subj = new Subject();
  MockInstance(ApiService, 'save', () => subj);

  const apiService = TestBed.inject(ApiService);
  subj.next(data);
});

[DmitryEfimenko]

There are two reasons why I came up with my utility function:

• avoid boilerplate associated with creating a new Subject (multiplied by the number of methods that need to be stubbed. In my recent test I had to stub three methods)
• make it a bit more ergonomic. In your example, you called your Subject variable: subj. In the situation with multiple methods to stub, these Subjects start becoming something like myMethod1Subj etc. Whereas with the utility function I came up with it's all encapsulated under one object: flush and the subjects follow exact method names of the methods they stub. Convenient.

Going back to my example:

const deps = MockBuilder().mock(ApiService).build();
TestBed.configureTestingModule(deps);
const apiService = TestBed.inject(ApiService);

You have pointed out the problem:

The main problem here is that apiService.save doesn't know anything about flushSuccess

As a matter of fact, not only does it not know about flushSuccess, but it also does not know anything about the fact that that service has been mocked.

Calling const apiService = TestBed.inject(ApiService) will return a variable of type ApiService - not jasmine.SpyObj<MyService>.
So, writing something like apiService.save.and.returnValue(subject) will also be a problem. One needs to explicitly cast apiService to jasmine.SpyObj. But if that's the case, we might as well come up with our own type which is a combination of jasmine.SpyObj<MyService> and something that knows of flushSuccess. That's what spectator does.

In order to get around the casting issue we could come up with a method on ngMocks object. Something like:

ngMocks.injectMocked(ApiService)

[satanTime]

Agree, that's a good point, that's why I usually go with MockInstance or ngMocks.stubMember to override spied methods to return what I need regardless how i was spied initially, and default spied methods are used for assertions only.

Something like:

ngMocks.autoSpy('jasmine');

const deps = MockBuilder(UserService, ApiService).build();
TestBed.configureTestingModule(deps);

const userService = TestBed.inject(UserService); // real
const apiService = TestBed.inject(ApiService); // mock

// an override of a spy to return what I need
ngMocks.stubMember(apiService, 'loadConfig', () => of(...));

// calls apiService.loadConfig and apiService.userSave
userService.save();

// doesn't fail because apiService.post is a spy
expect(apiService.userSave).toHaveBeenCalledWith(...);

Also, another thing is that it's not very clear whether ngMocks.injectMocked returns a spy from jasmine or jest or none.

Anyway for observables, it's just a Subject and I don't see a problem to inject it into any spy. We just need to find a good way to do it.

[DmitryEfimenko]

right. This approach works for cases when you don't need to control the timing of the observable emitting. Recently I was writing a unit test where I actually needed to be in control of that.

Regarding the spy from jasmine or jest... yeah tricky issue! Spectator solves it with separate import paths. Not sure if similar approach would be suitable for ng-mocks. It would certainly be a breaking change.

[satanTime]

Could you share that test? I would like to see a real use-case.

[DmitryEfimenko]

I can't share exactly the use-case since it was work related. However, the approximate use-case is following:
Component under test is a file-Upload component that implements ControlValueAccessor.
Upon selecting a file it's supposed to:

• make a request to the API
• using the response of the first API call make a second call to upload a file
• after second API call succeeds, start polling 3rd API endpoint and wait for a particular response
  The user has ability to cancel file upload at any time.
  During each calls to API the component displays certain feedback to the user.

The way I would test some of the behaviors is something like (pseudocode):

// I kind of like this syntax with explicit `.keep()`, `.mock()` calls. It makes it very clear what's what
const deps = MockBuilder()
    .keep(FormsModule)
    .mock(SomeOtherComponent)
    .mock(MatProgressBar)
    .build();

// I use ng-mocks for mocking and spectator for testing environment
const createHost = createComponentFactory({
  component: MyComponent,
  ...deps,
});

// SIFERS
function setup() {
  // if the proposed feature was in place, I would not need to write this:
  const apiService = createSpyFromClass(ApiService);

  // if the proposed feature was in place, I would not need to write this:  
  const flush = createObservableFlushTriggers(
    apiService,
    'apiCall1',
    'apiCall2',
    'apiCall3'
  );
  
  const spectator = createHost({
    // if the proposed feature was in place, I would not need to write this:
    providers: [{ provide: ApiService, useValue: apiService }],
  });
  
  const pageObject = getPageObject(spectator);
  
  return {
    spectator,
    flush,
    apiService,
    pageObject
  };
}

function getPageObject(spectator: Spectator<MyComponent>) {
  const selectFile = () => {
    // fake implementation of selecting a file
    spectator.detectChanges();
  }
  
  const pressCancel = () => spectator.click('button[aria-label="cancel"]');
  
  return {
    selectFile,
    pressCancel
  };
}

it('should be able to cancel request after second API call started', () => {
  const { apiService, flush, pageObject } = setup();
  
  pageObject.selectFile();
  
  // only first API call started
  expect(apiService.apiCall1).toHaveBeenCalled();
  expect(apiService.apiCall2).not.toHaveBeenCalled();
  
  // first API call returned data
  flush.apiCall1.success({ /* some object */ });
  
  // second API call started
  expect(apiService.apiCall2).toHaveBeenCalled();
  
  pageObject.pressCancel();
  
  expect(pageObject.certainStateThatShouldBeTrueAfterCancel).toBe(true);
});

[DmitryEfimenko]

As a workaround for an issue of identifying all the methods that return an observable, we could perhaps use global configuration:

ngMocks.defaultConfig(MyService, {
  observablePropsToSpyOn: ['apiCall1', 'apiCall2', 'apiCall3'],
});

jasmine-auto-spies does something like that.

[satanTime]

Hi there, that's a good idea.

[kmjungersen]

I am looking to do something similar to this feature request (I believe), but let me know if this is separate and/or unrelated and I can open a new ticket.

I love this library, as it really speeds up the process of writing / maintaining tests! There's one thing I'm not sure how to accomplish though, as illustrated by the following example with Jasmine:

const someServiceSpy = jasmine.createSpyObj('SomeService', ['someMethod']);

TestBed.configureTestingModule({
    providers: [someServiceSpy],
    // ...
});

const serviceToTest = TestBed.inject(ServiceToTest);

it('should call someMethod', () => {
    serviceToTest.foo();

    expect(someServiceSpy.someMethod)
        .toHaveBeenCalled();
});

I really just want to make sure that the method someMethod is called (or called n times, or not called, etc.) by the service I'm testing (in method foo()). Is this possible, or is this the feature being requested?

Based on this comment above, it looks like this is the proposal for how this could be handled, but I'm just clarifying.

[kmjungersen]

Update: I ended up using a "hybrid" approach to accomplish this by creating a Jasmine SpyObj and using that as the Mocked instance, however it doesn't feel as clean.

const someServiceSpy: jasmine.SpyObj<SomeService>;

beforeEach(() => {
    someServiceSpy = jasmine.createSpyObj('SomeService', ['someMethod']);
    someServiceSpy.someMethod.and.returnValue('some-value');
    
    return MockBuilder(ServiceToTest, TestModule)
        .mock(SomeService, someServiceSpy);
});

it('should call someMethod', () => {
    serviceToTest.foo();

    expect(someServiceSpy.someMethod.calls.count())
        .toBe(1);
});

[satanTime]

Hi @kmjungersen, I think what you want is actually a combination of auto spy and MockBuilder for testing providers.
There is an example based on your code: https://codesandbox.io/s/aged-framework-gse2hs?file=/src/test.spec.ts

Please let me know if it does what you expect.

[kmjungersen]

Hi @satanTime - appreciate you looking into this! I tried following that path and that works for checking to see if the method has been called, which is great.

However the issue is when I attempt to actually stub a method for that service. Doing so makes the Mocked service function correctly, however then someServiceSpy ends up as a function and not a spy, if that makes sense.

I extended the sandbox you started to illustrate this: https://codesandbox.io/s/practical-kowalevski-vsfxgb?file=/src/test.spec.ts.

In reality SomeService is a service that calls an API, so I just have a method stub that returns a static value to mimic an API call in someMethod. So then in ServiceToTest I want to make sure a) the API service (SomeService) is called, and b) ServiceToTest applies the correct business logic to said value.

[kmjungersen]

Ah, I've got it! I didn't full understand how MockInstance played into everything until I saw the advanced example in the MockInstance docs.

Here's an updated Sandbox that passes now: https://codesandbox.io/s/sad-feynman-dp8vq2?file=/src/test.spec.ts

Thank you for pointing me in the right direction!

[satanTime]

Hi @kmjungersen,

thanks for the info.

With your requirements, I would simply create a spy instead of a stub function in MockBuilder or in MockInstance:

    MockBuilder(ServiceToTest, SomeService).mock(
      SomeService,
      {
        someMethod: jasmine.createSpy().and.callFake((x: string) => x),
      }
    )

Anyway, that's good that you've found a solution.

[rklec]

I agree mocking observables can be impr0oved in ng-mocks. I read your current guide, but even the "permanent fix" is not good enough IMHO.

Currently

• if auto-spy is off, you seem to mock Observable<..> properties as undefined. This causes issues, because the required properties are not there aka e.g. "Failed: can't access property "subscribe"".
• if auto-spyis on, it uses a Spy in jasmine e.g. - or actually stil undefined 8in my testing?)`? Anyway..

Problem

As said, and as you described in https://ng-mocks.sudo.eu/extra/mock-observables/ this is no good.

The problem with the permannent fix is I still have to define this for each service or component that uses this. This is exactly what I want to avoid and why I use `ng-mocks´ in the first place - I want to avoid to define all the stubbing manually and want to use sensitive defaults.

Proposed solution

Speaking about defaults, why not just stub out every Observable with EMPTY by default? Or at least make it configurable?

Something like:

ngMocks.defaultMockOptions(() => ({
  observables: EMPTY,
  observableMethods: () => EMPTY,
}));

I could still customize my stubs then or return something different, but it would be defaults that work/are enough in 99% of the cases.
That would be awesome! And totally solve my issues....

Even if you would have to assume each property ending in $ is an Observable that would be a big step IMHO. (as an optional feature, otherwise it could be a little unexpected)