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

feat($exceptionHandlerProvider): add mode logAndRethrow #10564

Closed
DavidSouther wants to merge 2 commits into from
Closed

feat($exceptionHandlerProvider): add mode logAndRethrow #10564

DavidSouther wants to merge 2 commits into from

Conversation

@DavidSouther
Copy link
Contributor

@DavidSouther DavidSouther commented Dec 23, 2014

Mode logAndRethrow captures the logged exception for later inspection by test
assertions, while also throwing the exception for when the implementation
depends on thrown behavior (eg leaving the method early).

Depends on #10563
Closes #10540

@DavidSouther
fix($exceptionHandlerProvider): call inject() to run tests
50c2902 
In the current angular-mocksSpec, the tests for $exceptionHandlerProvider
call `module` to run tests on `$exceptionHandlerProvider.mode()`, but do
not call `inject()` to pump the module definitions.
@petebacondarwin
Copy link
Contributor

petebacondarwin commented Dec 23, 2014

I think I agree that we should just log inside rethrow, since it won't do any harm and adding yet another config level seems unattractive and unnecessary.

@DavidSouther
Copy link
Contributor Author

DavidSouther commented Dec 23, 2014

I think I agree

:) I'll redo it that way!

@DavidSouther
feat($exceptionHandlerProvider): Log errors when rethrowing.
98a709c 
Current behavior draws a distinction between `log` and `rethrow` modes of
$exceptionHandler in ngMocks. This unifies the behaviors, with both modes
logging the errors while the `log` mode does not throw.

Closes #10540
petebacondarwin
petebacondarwin reviewed Dec 23, 2014
View reviewed changes
src/ngMock/angular-mocks.js
Copy link
Contributor

@petebacondarwin petebacondarwin Dec 23, 2014

Choose a reason for hiding this comment

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

typo

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Dec 23, 2014

@DavidSouther - thanks for this.
This looks good. I will fix the typo when merging.
The commit message needs just a tiny bit of tweaking:

  • No uppercase letter at the start of the description
  • No full stop at the end of the first line

which I will also fix when merging

petebacondarwin pushed a commit that referenced this pull request Dec 23, 2014
@DavidSouther @petebacondarwin
feat(ngMock/$exceptionHandler): log errors when rethrowing
2b97854 
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 pushed a commit that referenced this pull request Dec 23, 2014
@DavidSouther @petebacondarwin
feat(ngMock/$exceptionHandler): log errors when rethrowing
a4a4b82 
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
@DavidSouther
Copy link
Contributor Author

DavidSouther commented Dec 23, 2014

@petebacondarwin The branch is closed - did you want me to fix the commit message, typo, and ask for reopen?

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Dec 23, 2014

I merged it into 1.3.x and 1.4.x already

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Dec 23, 2014

I closed the PR by mentioning it in the commit that landed in master

@DavidSouther
Copy link
Contributor Author

DavidSouther commented Dec 23, 2014

Gotcha! I see where the merge happened; I'm just so used to seeing the
purple Github "merged" icon :) Thanks for your help, and thanks for taking
this change!

On Tue Dec 23 2014 at 2:25:56 PM Pete Bacon Darwin notifications@github.com
wrote:

I closed the PR by mentioning it in the commit that landed in master

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

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Dec 24, 2014

Yes, in Angular we don't use GitHub's merge as we rather have a linear git history.
We always rebase and do fast forward merges only (and cherry pick to other version branches).
By adding the "Closes #XXX" phrase to the bottom of the commit we can trigger Github to automatically close the PR. Perhaps we should ask them to support a "Merges #XXX" tag too?

@petems petems mentioned this pull request Dec 2, 2015
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

angular.mock.$ExceptionHandlerProvider Provide mode logAndRethrow

3 participants

@DavidSouther @petebacondarwin @googlebot