New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

angular.mock.$ExceptionHandlerProvider Provide mode `logAndRethrow` #10540

Closed
DavidSouther opened this Issue Dec 21, 2014 · 8 comments

Comments

Projects
None yet
3 participants
@DavidSouther
Contributor

DavidSouther commented Dec 21, 2014

https://github.com/angular/angular.js/blob/master/src/ngMock/angular-mocks.js#L253

Currently, test exceptions can either log or rethrow. There are cases where the throw behavior is expected by the implementation, but it would also be nice to have a log available. In these situations, a logAndThrow behavior would ease having two different code flows.

@petebacondarwin

This comment has been minimized.

Show comment
Hide comment
@petebacondarwin

petebacondarwin Dec 22, 2014

Member

You can easily achieve this via a decorator: http://plnkr.co/edit/OORm4yDYh2Gx1QoEvQt8

Member

petebacondarwin commented Dec 22, 2014

You can easily achieve this via a decorator: http://plnkr.co/edit/OORm4yDYh2Gx1QoEvQt8

@caitp

This comment has been minimized.

Show comment
Hide comment
@caitp

caitp Dec 22, 2014

Contributor

Actually, I think this would be good to have. It's ngMock so code size doesn't really matter, and there are a number of places where it's useful in core tests and worked around really awkwardly

Contributor

caitp commented Dec 22, 2014

Actually, I think this would be good to have. It's ngMock so code size doesn't really matter, and there are a number of places where it's useful in core tests and worked around really awkwardly

@petebacondarwin

This comment has been minimized.

Show comment
Hide comment
@petebacondarwin

petebacondarwin Dec 22, 2014

Member

Oh right! I didn't notice that it was angular-mocks. Fine by me.

This is an easy first PR if anyone is interested. Just add a new option to the mode and provide suitable tests (https://github.com/angular/angular.js/blob/master/test/ngMock/angular-mocksSpec.js#L589) and documentation (https://github.com/angular/angular.js/blob/master/src/ngMock/angular-mocks.js#L246).

Member

petebacondarwin commented Dec 22, 2014

Oh right! I didn't notice that it was angular-mocks. Fine by me.

This is an easy first PR if anyone is interested. Just add a new option to the mode and provide suitable tests (https://github.com/angular/angular.js/blob/master/test/ngMock/angular-mocksSpec.js#L589) and documentation (https://github.com/angular/angular.js/blob/master/src/ngMock/angular-mocks.js#L246).

@DavidSouther

This comment has been minimized.

Show comment
Hide comment
@DavidSouther

DavidSouther Dec 22, 2014

Contributor

I'll take a shot at it :)

On Mon Dec 22 2014 at 4:54:39 AM Pete Bacon Darwin notifications@github.com
wrote:

Oh right! I didn't notice that it was angular-mocks. Fine by me.

This is an easy first PR if anyone is interested. Just add a new option to
the mode and provide suitable tests (
https://github.com/angular/angular.js/blob/master/test/ngMock/angular-mocksSpec.js#L589)
and documentation (
https://github.com/angular/angular.js/blob/master/src/ngMock/angular-mocks.js#L246
).


Reply to this email directly or view it on GitHub
#10540 (comment)
.

Contributor

DavidSouther commented Dec 22, 2014

I'll take a shot at it :)

On Mon Dec 22 2014 at 4:54:39 AM Pete Bacon Darwin notifications@github.com
wrote:

Oh right! I didn't notice that it was angular-mocks. Fine by me.

This is an easy first PR if anyone is interested. Just add a new option to
the mode and provide suitable tests (
https://github.com/angular/angular.js/blob/master/test/ngMock/angular-mocksSpec.js#L589)
and documentation (
https://github.com/angular/angular.js/blob/master/src/ngMock/angular-mocks.js#L246
).


Reply to this email directly or view it on GitHub
#10540 (comment)
.

@caitp caitp reopened this Dec 22, 2014

@petebacondarwin

This comment has been minimized.

Show comment
Hide comment
@petebacondarwin

petebacondarwin Dec 22, 2014

Member

@DavidSouther - Great!

Please make sure that you have signed the CLA, associated it with your Github username and that your commit messages match our requirements.

Get in touch if you need any help.

Member

petebacondarwin commented Dec 22, 2014

@DavidSouther - Great!

Please make sure that you have signed the CLA, associated it with your Github username and that your commit messages match our requirements.

Get in touch if you need any help.

@DavidSouther

This comment has been minimized.

Show comment
Hide comment
@DavidSouther

DavidSouther Dec 23, 2014

Contributor

Uh... none of these module(function($exceptionHandlerProvider){}) tests are actually running.

Contributor

DavidSouther commented Dec 23, 2014

Uh... none of these module(function($exceptionHandlerProvider){}) tests are actually running.

@DavidSouther

This comment has been minimized.

Show comment
Hide comment
@DavidSouther

DavidSouther Dec 23, 2014

Contributor

The module() calls did not have a corresponding inject(), so no assertions were executed. #10563 fixes that.

Contributor

DavidSouther commented Dec 23, 2014

The module() calls did not have a corresponding inject(), so no assertions were executed. #10563 fixes that.

@DavidSouther

This comment has been minimized.

Show comment
Hide comment
@DavidSouther

DavidSouther Dec 23, 2014

Contributor

For a larger conversation, does it make sense to combine logAndRethrow into rethrow? Personally, that was my expected behavior before reading the docs that the additional logic from $exceptionHandler in mocks would only add, not change, functionality.

Contributor

DavidSouther commented Dec 23, 2014

For a larger conversation, does it make sense to combine logAndRethrow into rethrow? Personally, that was my expected behavior before reading the docs that the additional logic from $exceptionHandler in mocks would only add, not change, functionality.

petebacondarwin added a commit that referenced this issue Dec 23, 2014

feat(ngMock/$exceptionHandler): log errors when rethrowing
Now the `rethrow` mode will also record a log of the error in the same
way as the `log` mode.

Closes #10540
Closes #10564

petebacondarwin added a commit that referenced this issue Dec 23, 2014

feat(ngMock/$exceptionHandler): log errors when rethrowing
Now the `rethrow` mode will also record a log of the error in the same
way as the `log` mode.

Closes #10540
Closes #10564

Conflicts:
	src/ngMock/angular-mocks.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment