67

I'm testing a component which subscribe router params. Every test pass and everything works fine. But if I look in the console, I can see an error:

Error during cleanup of component ApplicationViewComponent localConsole.(anonymous function) @ context.js:232

Do you know why this occurs?

I tried removing the unsubscribe() from ngOnDestroy() method and the error disappears.

Is karma/jasmine supporting unsubscribe() automatically?

Here is the component and tests

Component

import { Component, OnInit } from '@angular/core';   
import { ActivatedRoute } from '@angular/router';
import { Subscription } from 'rxjs/Rx'

import { AppService } from 'app.service';

@Component({
  selector: 'app-component',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss']
})
export class AppComponent implements OnInit {
  private routeSubscription: Subscription;

  // Main ID
  public applicationId: string;


  constructor(
    private route: ActivatedRoute,
    private _service: AppService
  ) { }

  ngOnInit() {
    this.routeSubscription = this.route.params.subscribe(params => {
      this.applicationId = params['id'];

      this.getDetails();
      this.getList();
    });
  }

  getDetails() {
    this._service.getDetails(this.applicationId).subscribe(
      result => {     
        console.log(result);
      },
      error => {  
        console.error(error);        
      },
      () => {
        console.info('complete');
      }
    );
  }

  getList(notifyWhenComplete = false) {
    this._service.getList(this.applicationId).subscribe(
      result => {     
        console.log(result);
      },
      error => {  
        console.error(error);        
      },
      () => {
        console.info('complete');
      }
    );
  }

  ngOnDestroy() {
    this.routeSubscription.unsubscribe();
  }

}

Component spec file

import { NO_ERRORS_SCHEMA } from '@angular/core';
import {
  async,
  fakeAsync,
  ComponentFixture,
  TestBed,
  tick,
  inject
} from '@angular/core/testing';
import {
  RouterTestingModule
} from '@angular/router/testing';
import {
  HttpModule
} from '@angular/http';
import { Observable } from 'rxjs/Observable';
import { Router, ActivatedRoute } from '@angular/router';

// Components
import { AppComponent } from './app.component';

// Service
import { AppService } from 'app.service';
import { AppServiceStub } from './app.service.stub';

let comp:    AppComponent;
let fixture: ComponentFixture<AppComponent>;
let service: AppService;

let expectedApplicationId = 'abc123';

describe('AppComponent', () => {
  beforeEach(async(() => {
    TestBed.configureTestingModule({
      declarations: [AppComponent],
      imports: [RouterTestingModule, HttpModule],
      providers: [
        FormBuilder,
        {
          provide: ActivatedRoute,
          useValue: {
            params:  Observable.of({id: expectedApplicationId})
          }
        },
        {
          provide: AppService,
          useClass: AppServiceStub
        }    
      ],
      schemas: [ NO_ERRORS_SCHEMA ]
    })
    .compileComponents();
  }));

  tests();
});

function tests() {
  beforeEach(() => {
    fixture = TestBed.createComponent(AppComponent);
    comp = fixture.componentInstance;

    service = TestBed.get(AppService);
  });


  /*
  *   COMPONENT BEFORE INIT
  */
  it(`should be initialized`, () => {
    expect(fixture).toBeDefined();
    expect(comp).toBeDefined();
  });


  /*
  *   COMPONENT INIT
  */

  it(`should retrieve param id from ActivatedRoute`, async(() => {
    fixture.detectChanges();

    expect(comp.applicationId).toEqual(expectedApplicationId);
  }));

  it(`should get the details after ngOnInit`, async(() => {
    spyOn(comp, 'getDetails');
    fixture.detectChanges();

    expect(comp.getDetails).toHaveBeenCalled();
  }));

  it(`should get the list after ngOnInit`, async(() => {
    spyOn(comp, 'getList');
    fixture.detectChanges();

    expect(comp.getList).toHaveBeenCalled();
  }));
}

service.stub

import { Observable } from 'rxjs/Observable';

export class AppServiceStub {
  getList(id: string) {
    return Observable.from([              
      {
        id: "7a0c6610-f59b-4cd7-b649-1ea3cf72347f",
        name: "item 1"
      },
      {
        id: "f0354c29-810e-43d8-8083-0712d1c412a3",
        name: "item 2"
      },
      {
        id: "2494f506-009a-4af8-8ca5-f6e6ba1824cb",
        name: "item 3"      
      }
    ]);
  }
  getDetails(id: string) {
    return Observable.from([      
      {        
        id: id,
        name: "detailed item 1"         
      }
    ]);
  }
}
BlackHoleGalaxy
  • 9,160
  • 17
  • 59
  • 103

14 Answers14

105

The "Error during component cleanup" error message happens because when ngOnDestroy() is called, this.routeSubscription is undefined. This happens because ngOnInit() was never invoked, meaning that you never subscribed to the route. As described in the Angular testing tutorial, the component isn't initialized fully until you call fixture.detectChanges() the first time.

Therefore, the correct solution is to add fixture.detectChanges() to your beforeEach() block right after the createComponent is called. It can be added any time after you create the fixture. Doing so will ensure that the component is fully initialized, that way component cleanup will also behave as expected.

randomPoison
  • 1,310
  • 1
  • 9
  • 13
  • The fact is I dont't necessarily want the `ngOnInit` to be invoked in my test. – BlackHoleGalaxy May 25 '17 at 23:21
  • 3
    @BlackHoleGalaxy any reason why not? If you're not fully initializing the component when you test it, then your tests aren't going to accurately test the behavior of the component. – randomPoison May 26 '17 at 18:58
  • It's in case I want to test a method in my component outside the context of the component itself. Pure unit test. After that I test the method from the component context. I separate my unit tests from intergrations ones. – BlackHoleGalaxy May 27 '17 at 13:06
  • 4
    @BlackHoleGalaxy In that case you probably shouldn't create the fixture with `TestBed`. If I recall correctly, `TestBed` automatically calls `ngOnDestoy` on your component when the unit test completes, so you need to make sure the component is fully initialized as part of the test. If your component has a static method then you can unit test it separately without creating a fixture, but methods that require an instance should be tested on a fully initialized instance of the component. – randomPoison May 28 '17 at 15:32
  • 22
    in my test cases `fixture.detectChanges()` is added still I am getting the error – Aniruddha Das Jun 06 '17 at 22:01
  • @AniruddhaDas Could you post your example code somewhere or open a separate question? For OP's situation, `fixture.detectChanges()` ensures that `ngOnInit()` is called and that the subscription is created. If your situation is different, then a different solution might be necessary. – randomPoison Jun 07 '17 at 16:16
  • In my case, I get this error because the spy I'm creating to intercept the call throwing the error (expected behaviour in the context of the test) inside `ngOnDestroy` is not working. Even simpler, this is not working: `spyOn(component, 'ngOnDestroy')`. – aaaaahaaaaa May 25 '18 at 16:40
  • Works for me, Angular 5 – Jasmeet Aug 16 '18 at 18:31
  • 1
    @AniruddhaDas this error can also be triggered in tests of AComponent, where AComponent component is creating BComponent and BComponent is using unsubscribe in ngOnDestroy – Bogdan D Feb 17 '20 at 15:33
  • @Bogdan D, agree – Aniruddha Das Feb 17 '20 at 16:31
  • do it like this ` fixture = TestBed.createComponent(HomeComponent); component = fixture.componentInstance; fixture.detectChanges() ` if you add `fixture.detectChanges()` before `component=` you will receive errors – imnickvaughn May 14 '20 at 09:28
43

You need to refactor your method ngOnDestroy as below :

ngOnDestroy() {
  if ( this.routeSubscription)
    this.routeSubscription.unsubscribe();
}
musecz
  • 797
  • 7
  • 17
  • 11
    Altering your code only for tests to work usually hide a problem ... I think @excaliburHisShealth answer should be the accepted one – j3ff Jul 28 '17 at 14:10
  • 1
    @j3ff Which answer is that? I don't see any "excaliburHisShealth" answers here. Could you link to the answer in your comment? – Wilt Apr 08 '22 at 19:10
12

In my case destroying the component after each test solved the problem. So you could try adding this to your describe function:

afterEach(() => {
  fixture.destroy();
})
Alex Link
  • 1,210
  • 13
  • 16
8

I'm in a similar situation where I want to test a function in my component outside the context of the component itself.

This is what worked for me:

afterEach(() => {
  spyOn(component, 'ngOnDestroy').and.callFake(() => { });
  fixture.destroy();
});
Richard Medeiros
  • 938
  • 9
  • 18
7

So my situation was similar, but not exactly the same: I'm just putting this here in case someone else finds it helpful. When unit testing with Jamine/Karma I was getting

 'ERROR: 'Error during cleanup of component','

It turns out that was because I wasn't properly handling my observables, and they didn't have an error function on them. So the fix was adding an error function:

this.entityService.subscribe((items) => {
      ///Do work
},
  error => {
    this.errorEventBus.throw(error);
  });
5

You can add

teardown: { destroyAfterEach: false },

in the TestBed.configureTestingModule, it will look like this

beforeEach(async () => {
        await TestBed.configureTestingModule({
            declarations: [<Your component here>],
            imports: [<Your imports here>],
            providers: [<Your providers here>],
            teardown: { destroyAfterEach: false },
        }).compileComponents();
    });
Rossi Alex
  • 81
  • 1
  • 2
3

Adding to @David Brown's response the code below is what worked for me.

      .subscribe(res => {
          ...
        },
        error => Observable.throw(error)
      )
Petros Kyriakou
  • 5,214
  • 4
  • 43
  • 82
3

As explained by @randomPoison, the error is triggered when the component that uses unsubscribe is not initialised. However, calling fixture.detectChanges() is a solution for when the error is in the spec file of the respective component.

But we might also be dealing with a FooComponent that creates BarComponent and BarComponent uses unsubscribe in its ngOnDestroy. Proper mocking must be done.

I'd suggest a different approach to the subscription cleanup, one that is declarative and won't trigger such problems. Here's an example:

export class BazComponent implements OnInit, OnDestroy {
  private unsubscribe$ = new Subject();

  ngOnInit(): void {
    someObservable$
      .pipe(takeUntil(this.unsubscribe$))
      .subscribe(...);
  }

  ngOnDestroy(): void {
    this.unsubscribe$.next();
    this.unsubscribe$.complete();
  }
}

More on this approach here

Bogdan D
  • 5,321
  • 2
  • 31
  • 32
2

You have to do 2 things, to solve this error.

1- add fixture.detectChanges(); in beforeEach()
2 - you need to add below, so that component can be clear.

afterEach(() => {
        fixture.destroy();
      });
0

Well in my case the error was in the template. There was error in the child component ngDestroy ,which wasn't getting destroyed as i was trying to set readonly property. It would be worth your time checking your child components whether they are getting destroyed properly.

Vikhyath Maiya
  • 3,122
  • 3
  • 34
  • 68
0

For me what fixed this error was inside of my component's ngOnDestroy, I wrapped my store dispatch and my unsubscribe in a try catch.

ngOnDestroy(): void {
 try {
  this.store.dispatch(new foo.Bar(this.testThing()));
  if(this.fooBarSubscription) {
   this.fooBarSubscription.unsubscribe();
  }
 } catch (error) {
   this.store.dispatch(new foo.Bar(this.testThing()));
  }
}
Papa_D
  • 11
  • 2
0

In my case I was testing a component with multiple @Input properties. I had to set it inside beforeEach block to [] component.xyz = [] (since it was type of an array). That was the origin of an problem.

Chaka15
  • 1,105
  • 1
  • 8
  • 24
0

I got this error message when I was setting one of the services variables to undefined. That variable holds the information (id) of currently active entity that is being edited. In that setter method I check that if the parent entity is set ... and that caused the error, because my tests don't set that parent entity at any point.

so the code looked like this

 ngOnDestroy(): void {
    this.subs.forEach(s => s.unsubscribe());
    this.xxxEditService.activeYYY = undefined; // setter checks that parent is set, otherwise throws an error
 }

I solved this by providing some mock entity in my test file

providers: [
 {
   provide: xxxEditService,
   useValue: {
     parentEntity: {} as Parent,
     activeYYY: -1   // id
   }
}

TLDR: Something crashes during the clean up (ie. null/undef references), and it might be because of non-valid presets

O-9
  • 1,626
  • 16
  • 15
0

I got this due to human error.

In my case, I forgot to add method sendC(a new method added to ABCComponent) to the list of methods of the dependent spy component. As an Example below

let abcComponent: jasmine.SpyObj<ABCComponent>
///
beforeEach(async () => {

    abcComponent = jasmine.createSpyObj('ABCComponent', [
      'sendA$',
      'sendB$',
      'sendC$' // initially forgot to add and I got the Failed: [ 'Error during cleanup of component' error. after adding this, my issue is fixed.
    ])

    await TestBed.configureTestingModule({
      declarations: [AComponent],
      providers: [
        AState,
        APipe,
        BPipe,
        
        { provide: ABCComponent, useValue: abcComponent },
        { provide: Router, useValue: routerSpy },
        { provide: NavigationService, useValue: navigationServiceSpy},
      ],
      imports: [ABCModule]
    })
      .compileComponents()
      abcComponent = TestBed.inject(ABCComponent) as jasmine.SpyObj<ABCComponent>
  })

So checking this solved my issue and also might solve yours.!!

Brooklyn99
  • 987
  • 13
  • 24