Skip to content

Conversation

@anthonn
Copy link
Contributor

@anthonn anthonn commented Nov 13, 2017

This pull request is to replace (#8404), please check this for more information.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 13, 2017
}

ngOnDestroy() {
super.ngOnDestroy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a unit test for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really tried to come up with a unit test, but I have not written unit tests for Jasmin/Karma before so I didnt really get it to work.

This is what I came up with but chrome hangs during execution:

it('make sure accordion item runs ngOnDestroy when expansion panel is destroyed', () => {
  let fixture = TestBed.createComponent(PanelWithContent);
  spyOn(fixture.componentInstance.panel.destroyed, 'emit');
  fixture.componentInstance.panel.ngOnDestroy();      
  fixture.detectChanges();
  expect(fixture.componentInstance.panel.destroyed.emit).toHaveBeenCalled();
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so I think I managed to make a correct unit test finally.

Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@josephperrott josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Nov 14, 2017
@jelbourn jelbourn merged commit f6bd9b0 into angular:master Nov 20, 2017
@anthonn anthonn deleted the fix-expansion-memory-leak branch May 10, 2018 15:32
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants