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.
- 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.
- 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.
- 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?
Does this look good to you ?
Regarding your last point
You can do that actually.
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.
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 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
I had exactly the same problem, here's my solution:
component.ts:
component.spec.ts:
This works for me, I hope it will for you too !