Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(ngMock): don't clean up if currentSpec is null. #16856

Open
wants to merge 1 commit into
base: v1.6.x
Choose a base branch
from

Conversation

heathkit
Copy link

@heathkit heathkit commented Mar 22, 2019

In unit tests that use Angular and AngularJS, sometimes an error being thrown during the test and cause this afterEach hook to run twice. If that happens, on the second run cleanup will throw an error, since at that point currentSpec is null. This causes the original error to be hidden by a "TypeError: Cannot read property '$injector' of null while testing" error.

This makes cleanup safe to call multiple times, working around the issue.

This fix is needed to resolve an internal issue affecting Angular unit tests. I discussed this with Igor, and he asked me to submit this as a PR. No release is necessary, we just need this in google3.

@heathkit heathkit changed the title fix(ngMocks): Don't clean up if currentSpec is null. fix(ngMock): Don't clean up if currentSpec is null. Mar 22, 2019
@heathkit heathkit force-pushed the v1.6.x branch 2 times, most recently from 91a91b6 to 79b8317 Compare March 22, 2019 04:15
@heathkit heathkit changed the title fix(ngMock): Don't clean up if currentSpec is null. fix(ngMock): don't clean up if currentSpec is null. Mar 22, 2019
@gkalpak
Copy link
Member

gkalpak commented Mar 22, 2019

I would feel better if we understood why this happens. Why is the afterEach() block run twice?

In unit tests that use Angular and AngularJS, sometimes an error being
thrown during the test and cause this afterEach hook to run twice. If
that happens, on the second run cleanup will throw an error, since at
that point currentSpec is null. This causes the original error to be
hidden by a "TypeError: Cannot read property '$injector' of null while
testing" error.

This makes cleanup safe to call multiple times, working around the
issue.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants