How to unit test unsubscribe function in angular

Question

I would like to find a way to test unsubscribe function calls on Subscriptions and Subjects.

I came up with a few possible solutions, but every one of these have pros and cons. Please keep in mind that I do not want to alter the access modifier of a variable for testing purposes.

1. Accessing private variable of component with reflection.

In that case I have a private class variable which stores a subscription:

component.ts:

private mySubscription: Subscription;
//...
ngOnInit(): void {
    this.mySubscription = this.store
        .select(mySelector)
        .subscribe((value: any) => console.log(value));
}

ngOnDestroy(): void {
    this.mySubscription.unsubscribe();
}

---

component.spec.ts:

spyOn(component['mySubscription'], 'unsubscribe');
component.ngOnDestroy();
expect(component['mySubscription'].unsubscribe).toHaveBeenCalledTimes(1);

---

pros:

• I can reach mySubscription.
• I can test that the unsubscribe method was invoked on the right subscription.

cons:

• I can reach mySubscription only with reflection, what I would like to avoid if possible.

---

2. I create a variable for subscription just like in option 1., but instead of reaching the variable with reflection I simply check that the unsubscribe method was invoked, without knowing the source.

component.ts: same as in option 1

---

component.spec.ts:

spyOn(Subscription.prototype, 'unsubscribe');
component.ngOnDestroy();
expect(Subscription.prototype.unsubscribe).toHaveBeenCalledTimes(1);

---

pros:

• I can test that the unsubscribe method was called

cons:

• I can not test the source of the invoked unsubscribe method.

---

3. I implemented a helper function which invokes unsubscribe method on the passed parameters which are subscriptions.

subscription.helper.ts:

export class SubscriptionHelper {

    static unsubscribeAll(...subscriptions: Subscription[]) {
        subscriptions.forEach((subscription: Subscription) => {
                subscription.unsubscribe();
            });
    }
}

---

component.ts: same as in option 1, but ngOnDestroy is different:

ngOnDestroy(): void {
    SubscriptionHelper.unsubscribeAll(this.mySubscription);
}

---

component.spec.ts:

spyOn(SubscriptionHelper, 'unsubscribeAll');
component.ngOnDestroy();
expect(SubscriptionHelper.unsubscribeAll).toHaveBeenCalledTimes(1);

---

pros:

• I can test that the helper function was called

cons:

• I can not test that the unsubscribe function was called on a specific subscription.

---

What do you guys suggest? How do you test the cleanup in unit test?

Seurat

Why do you want to test unsubscribe? this is a piece of rxjs and I think you can safely assume it works – Hugohugon

I think PO just wants to make sure that unsubscribe is getting called. – Escutcheon

I would like to test the component cleanup and not the behavior of unsubscribe. If I would like to test the behavior then I would check that the subscription which was the source of the unsubscribe call is not active any more. But again, I would like to test that the component calls unsubscribe on every subscription. – Seurat

Answer 1

I had exactly the same problem, here's my solution:

component.ts:

private subscription: Subscription;
//...
ngOnInit(): void {
    this.subscription = this.route.paramMap.subscribe((paramMap: ParamMap) => {
        // ...
    });
}

ngOnDestroy(): void {
    this.subscription.unsubscribe();
}

component.spec.ts:

let dataMock;

let storeMock: Store;
let storeStub: {
    select: Function,
    dispatch: Function
};

let paramMapMock: ParamMap;
let paramMapSubscription: Subscription;
let paramMapObservable: Observable<ParamMap>;

let activatedRouteMock: ActivatedRoute;
let activatedRouteStub: {
    paramMap: Observable<ParamMap>;
};

beforeEach(async(() => {
    dataMock = { /* some test data */ };

    storeStub = {
        select: (fn: Function) => of((id: string) => dataMock),
        dispatch: jasmine.createSpy('dispatch')
    };

    paramMapMock = {
        keys: [],
        has: jasmine.createSpy('has'),
        get: jasmine.createSpy('get'),
        getAll: jasmine.createSpy('getAll')
    };

    paramMapSubscription = new Subscription();
    paramMapObservable = new Observable<ParamMap>();

    spyOn(paramMapSubscription, 'unsubscribe').and.callThrough();
    spyOn(paramMapObservable, 'subscribe').and.callFake((fn: Function): Subscription => {
        fn(paramMapMock);
        return paramMapSubscription;
    });

    activatedRouteStub = {
        paramMap: paramMapObservable
    };

    TestBed.configureTestingModule({
       // ...
       providers: [
           { provide: Store, useValue: storeStub },
           { provide: ActivatedRoute, useValue: activatedRouteStub }
       ]
    })
    .compileComponents();
}));

// ...

it('unsubscribes when destoryed', () => {
    fixture.detectChanges();

    component.ngOnDestroy();

    expect(paramMapSubscription.unsubscribe).toHaveBeenCalled();
});

This works for me, I hope it will for you too !

Alpheus

clean comment, just needed method and what. – Tshombe

Answer 2

I figure out a way to make it work.

First of all, I do not create a subscription instance for each subscription, I usually use only one instance of subscription and add into as many subscriptions I need by using me method 'add' that implements a teardownLogic and allows child subscriptions.

private subscriptions: Subscription;
 //...
ngOnInit(): void {
  this.mySubscription.add(
    this.store
      .select(mySelector)
      .subscribe((value: any) => console.log(value))
  );
}

ngOnDestroy(): void {
  this.subscriptions.unsubscribe();
}

By doing that, in my tests I can rely on the Subscriptions prototype and detects the unsubscribe method calls. In that case after I call the ngOnDestroy.

const spy = spyOn(Subscription.prototype, 'unsubscribe');
component.ngOnDestroy();
expect(spy).toHaveBeenCalledTimes(1);

Dede

There's a small caveat with this method, which is that you don't really know that the unsubscribe that you're trying to test is the one that has actually been called. I agree that it works in most cases, but it's far from fool proof. – Alpheus

Answer 3

Does this look good to you ?

component.mySubscription = new Subscription();

spyOn(component.mySubscription, 'unsubscribe');
component.ngOnDestroy();

expect(component.mySubscription.unsubscribe).toHaveBeenCalledTimes(1);

Longford

In that case mySubscription is public, but it should not be. – Seurat

Oh sorry, I didn't read correctly. What you ask is a bit confusing because you want to test the identity of something that is by definition suppose to be unknown... What's the point to keep it private ? Is it more relevant than having it public ? – Longford

Answer 4

.component.ts:

...
private someSubscription!: Subscription;
...
ngOnInit(): void {
  this.someSubscription = ... .subscribe(() => ...);
}
...
ngOnDestroy(): void {
  this.someSubscription.unsubscribe();
}
...

.component.spec.ts:

...
it('should unsubscribe', () => {
  component['someSubscription'] = of().subscribe();
  const unsubscriptionSpy = jest.spyOn(component['someSubscription'], 'unsubscribe');
  component.ngOnDestroy();
  expect(unsubscriptionSpy).toHaveBeenCalled();
});
...

When the code inside ngOnDestroy (component) is modified, the test fails. Therefore, this solution was working fine for me.

Soho

Answer 5

I move away from COMMENTS, to gain some more space, even if mine is just an open reasoning. I hope this is acceptable by SO.

I would like to test that the component calls unsubscribe on every subscription

I think this is an interesting case of test design.

We all know that it is important not to leave subscriptions alive when a component is destroyed, and so it is worth thinking how to test this.

At the same time this could be seen as an implementation detail and it certainly does not belong to the "publicly accessible behavior" which a SW component exposes via its APIs. And public APIs should be the focus area of tests if you do not want to couple tightly tests and code, which is something that impedes refactoring.

Plus AFAIK RxJS does not provide a mechanism to list all active subscriptions at a given moment, so the task of finding a way to test this remains on us.

So, if you want to reach your objective of testing that all subscriptions are unsubscribed, you need to adopt a specific design to allow this, e.g. the use of an array where you store all of your subscriptions and then a check to see if all of them have been closed. This is the line of your SubscriptionHelper.

But even this does not necessarly put you in a safe place. You can always create a new subscription and not add it to the array, maybe simply because you forget to do this in your code.

So, you can test this only if you stick to your coding discipline, i.e. you add subscriptions to the array. But this is equivalent of making sure you unsubscribe all of your subscriptions in ngOnDestroy method. Even this is coding discipline.

So, does it really make sense to have such test even if the end objective of the test is important? Shouldn't it be something to acheive via code review?

Very curious to read opinions

Hugohugon

I am curious too. Just keep in mind: if we create an array to store subscriptions in it, we still have to expose it, which is not a good practice because of the shape of the API, or we have to gain access to this private member with reflection, what I would like to avoid. – Seurat

Just to be clear: I do not think this is something that should be tested by a test case, since it links too much implementation details with tests – Hugohugon

I think it should be tested, to make sure the memory is not leaking. – Seurat

Working TDD, it would be (nearly) impossible to add the unsubscribe call, without a test. That should suffice. If it does not for you, think about the fact that your code does something important, that you want to make sure nobody stops doing. That's worth testing by definition. It's not about coupling, it's about writing viable tests that do the job of verifying the correct behavior of the unit. Regarding the mechanism for testing it, see my answer above. – Alpheus

Answer 6

Regarding your last point

I can not test that the unsubscribe function was called on a specific subscription.

You can do that actually.

// get your subscrption
const mySubscription = component['mySubscription'];

// use the reference to confirm that it was called on that specific subscription only.
expect(SubscriptionHelper.unsubscribeAll).toHaveBeenCalledWith(mySubscription);

---

Now, when it comes to testing, it should just check/access/call the public members, and test/expect its effects.

Using the luxury of javascript, you are expecting/validating private members, even that is fine I think. Even I do that, I may be wrong. Productive criticism and comments are welcomed.

Escutcheon

At last, my two cents on this: Testing should not drive the development, however, our code should be testable. – Escutcheon

My intention with this question is exactly that, to clarify if this approach is widely used or not. Or maybe there is a better way to test things like that. – Seurat

So according to me, you are doing just fine. I provided answer for the point you were missing wrt method HaveBeenCalledWith. When it comes to accessing private variables (when there is no other way to verify the effect), I do it the same way to make sure the code is tested. well-explained question. +1 for that! – Escutcheon

"I can reach mySubscription only with reflection, what I would like to avoid if possible." Your proposed solution contains what I try to avoid in the first place. I am going to wait for other reviewers. Maybe they have other approach what we do not know yet. – Seurat

Answer 7

In Angular 16 we'll have access to a new token: DestroyRef.

With it, we can create a util function, which takes care of unsubscribing from Subscriptions:

export function destroyable<T>(observable: Observable<T>): Observable<T> {
  const subject = new Subject();
  inject(DestroyRef).onDestroy(() => {
    subject.next(true);
    subject.complete();
  });
  return observable.pipe(takeUntil(subject));
}

Usage in component:

ngOnInit(): void {
  destroyable(this.store.select(mySelector1))
    .subscribe((value: any) => console.log(value));
  destroyable(this.store.select(mySelector2))
    .subscribe((value: any) => console.log(value));
}

This solution is fully unit testable:

import * as util from '../util/destroyable';

it('should wrap interval in destroyable', () => {
  jest.spyOn(util, 'destroyable');

  fixture.detectChanges();
  expect(util.destroyable).toHaveBeenCalledWith(store.select(mySelector1));
});

I hope it helps you with dealing with Subscriptions.

Medium article.

Seurat

Answer 8

The most pragmatic approach is getting things into your hands and just use language feature.

const cmp = fixture.componentInstance as any;
cmp.mySubscription = jasmine.createSpyObj<Subscription>(['unsubscribe']);

Please remember it's absolutelly not a pattern for testing private code. You still have to test public pieces to indirectly prove the private code.

Legal

Answer 9

Lucien Dulac and Paritosh got it almost right:

component.ts:

private subscriptions = new Subscription();

constructor(): void {
    this.subscriptions.add(...);
}

ngOnDestroy(): void {
    this.subscriptions.unsubscribe();
}

component.spec.ts:

spyOn(component['subscriptions'], 'unsubscribe');
component.ngOnDestroy();
expect(component['subscriptions'].unsubscribe).toHaveBeenCalled();

Paymar