-
Notifications
You must be signed in to change notification settings - Fork 325
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
segregated getDefaultNode in utils #1005
base: master
Are you sure you want to change the base?
Conversation
import com.networknt.schema.JsonSchemaFactory; | ||
import com.networknt.schema.JsonSchemaRef; | ||
import com.networknt.schema.SpecVersion.VersionFlag; | ||
public class DefaultTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to DefaultsTest
.
public class DefaultTest { | ||
|
||
@Test | ||
void testGetDefaultNodeNotNull() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case is exactly the same as the testGetDefaultNodeEquals
case you have so you should remove this. If you want you can move the assertNotNull
here to that other test case, but I don't think that is needed.
You should test
- The behavior when the node cannot be found
- The behavior when there is a
$ref
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added node not found test case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still need the test for $ref
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
public class Defaults { | ||
|
||
public static JsonNode getDefaultNode(JsonSchema schema) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the formatting. There is additional spaces before public
and add the javadoc for the method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
import com.networknt.schema.JsonSchema; | ||
import com.networknt.schema.JsonSchemaRef; | ||
|
||
public class Defaults { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the javadoc for the class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
* Note: This class requires the 'com.networknt.schema.JsonSchema' and | ||
* 'com.networknt.schema.JsonSchemaRef' classes | ||
* from the 'networknt/json-schema-validator' library. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just change this to The 'Defaults' class provides utility methods for retrieving default values from a JSON schema.
@@ -31,4 +35,19 @@ void testGetDefaultNodeWhenDefaultNotFound() throws Exception { | |||
// Assert that the result is null, as there's no "default" node in the schema | |||
assertNull(result, "Default node should be null when 'default' node is not found in the schema"); | |||
} | |||
|
|||
@Test | |||
void testGetDefaultNodeWhenDefaultInRef() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't create external resources for the test. Just use strings inline as it makes the test hard to read and verify. This test isn't even valid. You have a unused variable JsonNode mainSchemaNode
.
Segregated
getDefaultNode
method to the utlis package to avoid duplicate code in the class.