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

test(errorHandlingConfig): add test for errorHandlingConfig() to be i… #15770

Closed
wants to merge 1 commit into from

Conversation

m-amr
Copy link
Contributor

@m-amr m-amr commented Mar 2, 2017

…ndependent of minErr

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
test

What is the current behavior? (You can also link to an open issue here)
There is no test for errorHandlingConfig
What is the new behavior (if this is a feature change)?
Add tests to errorHandlingConfig independent of minErr tests

Does this PR introduce a breaking change?
no

Please check if the PR fulfills these requirements

Other information:
These tests was added in #15707 , but it looks like there are some problems in the PR, so i have added the missing tests in this PR.

@m-amr m-amr force-pushed the test_error_handling branch from 21522fe to b80da97 Compare March 2, 2017 16:15
m-amr added a commit to m-amr/angular.js that referenced this pull request Mar 2, 2017
expect(errorHandlingConfig().objectMaxDepth).toBe(5);
});

it('should set objectMaxDepthx', function() {
Copy link
Member

Choose a reason for hiding this comment

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

objectMaxDepthx --> objectMaxDepth


it('should set objectMaxDepthx', function() {
errorHandlingConfig({objectMaxDepth: 3});
expect(errorHandlingConfig()).toEqual({objectMaxDepth: 3});
Copy link
Member

Choose a reason for hiding this comment

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

To make this test more robust, use expect(errorHandlingConfig().objectMaxDepth).toBe(3).
So, if we introduce more properties in the future, it won't break.

expect(errorHandlingConfig().objectMaxDepth).toBe(originalObjectMaxDepthInErrorMessage);
});

they('should set objectMaxDepth to NAN when $prop is supplied',
Copy link
Member

Choose a reason for hiding this comment

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

Nit: NAN --> Nan 😁

@gkalpak gkalpak added this to the Backlog milestone Mar 3, 2017
@m-amr m-amr force-pushed the test_error_handling branch from b80da97 to ed6269a Compare March 3, 2017 11:53
m-amr added a commit to m-amr/angular.js that referenced this pull request Mar 3, 2017
@m-amr
Copy link
Contributor Author

m-amr commented Mar 3, 2017

@gkalpak
Thanks for your feedback.
I have fixed the changes you requested.

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

One minor nit. LGTM otherwise 😃

expect(errorHandlingConfig().objectMaxDepth).toBe(originalObjectMaxDepthInErrorMessage);
});

they('should set objectMaxDepth to Nan when $prop is supplied',
Copy link
Member

Choose a reason for hiding this comment

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

I just realized I accidentally wrote Nan in my previous comment. I meant NaN 😁

@m-amr m-amr force-pushed the test_error_handling branch from ed6269a to eab5ec0 Compare March 3, 2017 12:39
@m-amr
Copy link
Contributor Author

m-amr commented Mar 3, 2017

@gkalpak
I have change it from Nan -> NaN.

@gkalpak gkalpak closed this in cc793a1 Mar 4, 2017
gkalpak pushed a commit that referenced this pull request Mar 4, 2017
ellimist pushed a commit to ellimist/angular.js that referenced this pull request Mar 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants