Unit testing responses to network requests

A fairly common idiom in my apps consists of:

  • pop a loading indicator
  • fire off a network request
  • when it resolves, dismiss the loading indicator

Here’s a framework I have come up with to test this flow. This assumes you’re using the vanilla jasmine/karma setup from angular-cli as a test harness environment. First, our mock network service. In ordinary operation, we want the network requests to just respond immediately. However, when we’re testing the loading controller flow, we need to be able to interrupt this. From a client (test) perspective, when we call freeze on our service, all subscriptions will block until we call unfreeze.

@Injectable()
export class RemoteMock {
  freezer: Subject<void>;
  remoteResource: Resource; // some business domain interface

  freeze(): void {
    this.freezer = new Subject<void>();
  }

  unfreeze(): void {
    this.freezer.next();
    this.freezer.complete();
    this.freezer = null;
  }

  freezeGuard<T>(f: () => T): Observable<T> {
    if (this.freezer) {
      return this.freezer.map(f);
    } else {
      return Observable.of(f());
    }
  }

  fetchResource(): Observable<Resource> {
    return this.freezeGuard(() => this.remoteResource);
  }
}

Here’s our component under test:

@Component()
export class ResourcePage {
  resource: Resource;

  constructor(private _backend: RemoteService, private _loadings: LoadingController) {
  }

  refresh(): void {
    let loading = this._loadings.create();
    this._backend.fetchResource().subscribe((rsrc) => {
      this.resource = rsrc;
    }, (err) => {
    }, () => {
      loading.dismiss();
    });
  }
}

Mocks that I use for LoadingController and friends are in this post. Now here’s our test:

describe('Page: Resource', () => {
  let fixture: ComponentFixture<ResourcePage>;
  let page: ResourcePage;

  beforeEach(fakeAsync(() => {
    TestBed.configureTestingModule({
      declarations: [ResourcePage],
      providers: [
        {provide: LoadingController, useClass: LoadingControllerMock},
        {provide: RemoteService, useClass: RemoteMock},
      ], 
    });
    fixture = TestBed.createComponent(ResourcePage);
    page = fixture.componentInstance;
    fixture.detectChanges();
    tick();
  }));

  it('should do loading controller stuff', fakeAsync(() => {
    page.refresh();
    let backend: RemoteMock = TestBed.get(RemoteService);
    backend.freeze();
    let loadings: LoadingControllerMock = TestBed.get(LoadingController);
    tick();
    expect(loadings.component.open).toBe(true);
    backend.unfreeze();
    tick();
    expect(loadings.component.open).toBe(false);
    expect(page.resource).toEqual(backend.remoteResource);
  }));
});

Comments and suggestions for improvement welcome.

1 Like

I would consider changing your larger mocked loader stuff into just jasmine spies:

const loadingSpy = jasmine.createSpyObj('loading', ['present', 'dismiss']);
const loadingControllerSpy = {
  create: jasmine.createSpy('create').and.returnValue(loadingSpy)
};

Which you could then check via:

expect(loadingController.create).toHaveBeenCalled();
expect(loadingSpy.present).toHaveBeenCalled();
expect(loadingSpy.dismiss).toHaveBeenCalled();

I find the whole freezing thing confusing I have to admit. Couldn’t you grab the backend and loadings variables up front, then call refresh. With refresh called you can check if loading was created/presented. Then call tick() which should advance to when the async call resolved. You should then be able to check the dismiss was called, and that you have the value from the service. There’s probably a reason that won’t work, but just by me looking at the code I don’t know what that is.

1 Like

Would that still be able to catch bugs like double-dismisses? I’m still paranoid about that sort of thing from my years of banging against memory allocation bugs in C++.

It is definitely confusing, and perhaps this isn’t the best case to explain why I think it’s needed. Typically mock services simply return Observables that fire immediately. I often have components that fire network requests in their constructors. Sometimes I have tests that don’t care when the fake network request resolves, and sometimes I have tests that want to checkpoint the point whereby the asynchronous request has been fired but not yet resolved. I struggled to come up with a way to serve both needs, and this is the best I’ve got to date.

Perhaps I’m being paranoid, but this doesn’t seem to be sufficient. I want to know that the loader was presented while the network request was outstanding, and dismissed once it resolved. These two expectations don’t seem to tell me what the situation was in between them, and that’s what I’m really trying to test.

Double dismisses: Yes, definitely, just tweak it a little:

expect(loadingSpy.dismiss.calls.count()).toEqual(1);

As far as timing, I would think it would be the same:

page.refresh();
expect(loadingSpy.create.calls.count()).toEqual(1); // create called only once because it's called immediately
expect(loadingSpy.dismiss).not.toHaveBeenCalled(); // dismiss hasn't been called yet because async hasn't finished
tick(); // This tick means the async call has resolved
expect(loadingSpy.dismiss.calls.count()).toEqual(1); // dismiss called once because async is finished

I feel like that covers both the multiple calls and the timing with far less code. These will actually fail the tests if dismiss is double called, where it seems like your mock object will just write to the console? It also keeps the logic of what you want to test (if dismiss get’s called too much) in the test, instead of in the mock object, which isn’t as good of a place for it.

1 Like

This is a great point that I am struggling with. How much logic do I put in my mocks which can be shared by all tests that use them, and how much do I put in the tests themselves?

Ya I wouldn’t put logic into a mock that the object it’s mocking doesn’t have, that’s going down a dangerous path. If double dismissal is a real life concern, you could consider writing a wrapper service for the loader that logs the error to the console on a double dismissal for the real service. Then your mock object is a bit more legitimate, however it still won’t fail the test if it’s called multiple times.

1 Like

Just an additional thought to this, if you have that pattern of showing and hiding a loading spinner with your requests, you could extend the Http service with a new one that handles the showing and hiding of the spinner. That would encapsulate that logic, and you could have just one unit test for your new service.

Also, in a bit of shameless self promotion, if you’re doing all that insanity to hack in the angular cli just to run tests, I just put out a new tutorial on a normal way to setup testing: http://www.roblouie.com/article/376/ionic-2-set-up-unit-testing-the-best-way/ :wink:

1 Like

I really dislike having services involve themselves with view-layer activity, although I realize this is a controversial stance.

Based on your feedback, I have modified my mock loading controllers to look like this:

export class LoadingComponentMock {
  free: boolean = true;
  open: boolean;
  errors: string[] = [];

  present(): void {
    if (this.free) {
      this.errors.push("attempt to present freed loading component");
    }
    if (this.open) {
      this.errors.push("loading component presented twice");
    }
    this.open = true;
  }

  dismiss(): void {
    if (!this.open) {
      this.errors.push("double-dismiss on loading component");
    }

    if (this.free) {
      this.errors.push("attempt to dismiss unallocated loading component");
    }

    this.open = false;
    this.free = true;
  }
}

export class LoadingControllerMock {
  component: LoadingComponentMock = new LoadingComponentMock();
  private _errors: string[] = [];

  create(): LoadingComponentMock {
    if (!this.component.free) {
      this._errors.push("can't have two loading components out at once");
      return null;
    }

    this.component.free = false;
    return this.component;
  }

  errors(): string[] {
    let rv = this._errors.concat(this.component.errors);
    return rv;
  }
}

Again, based on your feedback, this allows moving things into tests, so one can say:

expect(loadings.errors()).toEqual([]); 

Thank you for your suggestions.

1 Like