ngMock window.inject() does not add inject() call location info to errors thrown from repeated inject() callback calls

[jurko-gospodnetic]

Overview of the Issue
ngMock window.inject() function has some seemingly quick-fix functionality adding the window.inject() call location stack trace information to any errors thrown by the callback passed to the window.inject() call. This however has a bug in that it only adds this information to errors thrown from the first callback invocation, but not for any later ones, while there are typical scenarios when a callback given to it may be invoked multiple times during a single test run.

Motivation for or Use Case
Here's a typical scenario where a callback passed to ngMock's window.inject() function gets called multiple times in a single test suite:

describe('I\'m a little tea pot', function() {
  beforeEach(inject(function() {
    throw new Error();
  }));
  it('first test spec', function () {});
  it('second test spec', function () {});
});

Here the first test spec's error information will include the window.inject() call location stack trace information, while the second test spec's will not. And if for some reason only the second test spec throws an error, then no window.inject() call location stack trace information will be reported back to the user at all.

Angular Version(s)

• reproduced using angular 1.4.8 & master versions

Browsers and Operating System

• reproducible using any browser supporting JavaScript error stack information, e.g. Chrome, Firefox & Opera, IE10+ & PhantomJS
• reproduced using Ubuntu 15.10, but should not be OS specific

Reproduce the Error
Just run the use case presented in the Motivation for or Use Case section above.

Suggest a Fix

• don't clear the errorForStack information after the first call to the window.inject() callback

[jurko-gospodnetic]

Pull request #13597 is a fix for this issue for the 1.4.x branch.
Pull request #13598 is a fix for this issue for the master branch.

[jurko-gospodnetic]

ping?

[Narretz]

Thanks for raising this issue. It's not super high priority, so we'll look into it when we have the time.

[jurko-gospodnetic]

Heh, yeah... but clear pull requests #13597 & #13598 have been prepared resolving it so I don't think officially fixing this issue should take much time. And most of the changes in the pull requests are just added tests for previously completely untested part of the code-base. 😄

Kind of a low hanging fruit at the moment while, as the time goes by, the pull requests will just naturally collect conflicts.

There are actually two separate issues touching the same part of the code-base here - #13591 & this one - #13594. The pull requests for #13591 need to be applied first, as the pull requests for this issue build on those, adding to the same part of the test suite. And I think all the pull requests should be real easy to review + merge.:smile:

Please let me know if there is anything else I can do to get this merged.

Many thanks,
Jurko Gospodnetić

[Narretz]

Looks like the PRs have been merged!

[Narretz]

Oh no, the other one has been merged ...