Throw exception if max depth exceeded #161
Conversation
|
@bighappyface One of the test failed because of a connection timeout while "composer self-update" :-\ Is it possible to trigger this test again? see: https://travis-ci.org/justinrainbow/json-schema/jobs/67816242 |
| \JsonSchema\RefResolver::$maxDepth = 0; | ||
| $resolver = new \JsonSchema\RefResolver($retriever); | ||
|
|
||
| $this->assertEquals($jsonSchema, $resolver->resolve($jsonSchema)); |
There was a problem hiding this comment.
Why not just call the resolve method here without wrapping in an assertion? The @expectedException is the "assertion" for this test, no?
There was a problem hiding this comment.
Oh, you are absolutely right. Should I change it or should I leave it?
There was a problem hiding this comment.
@f-go yes, please refactor the test and rebase to a single commit.
@Maks3w @alecsammon would you mind helping with a quick review?
|
@f-go Travis is all green. Thanks for the test. |
|
@bighappyface Again the tests are failing due to a connection timeout :-/ |
|
@f-go the tests are green for me. Here is a good article for rebasing/squashing down a PR. We are close to getting this PR merged in, just the squash and a peer review. Thanks for your patience. |
|
@bighappyface Thanks for the guide and Sorry, this was my first PR. |
|
@f-go any chance to squash this PR down? |
|
done. sorry for the delay. |
|
+1 @localheinz @GrahamCampbell @Maks3w @alecsammon would you please take a look for community review? |
|
+1 |
Throw exception if max depth exceeded
|
@f-go good work! @alecsammon thanks for the help 😺 |
Rather than fail quietly, when the max depth is exceeded, an exception should be thrown. This tells very clearly what's wrong, and you do not spend hours searching for the fault in the schema.