fix/check_obligations_and_prohibitions#509
Conversation
tom-rm-meyer-ISST
left a comment
There was a problem hiding this comment.
Thanks a lot and great catch :) only had small findings and one point for discussion :)
| JsonNode validJsonNode = objectMapper.readTree(validJson); | ||
| JsonLdUtils jsonLdUtils = new JsonLdUtils(); | ||
| validJsonNode = jsonLdUtils.expand(validJsonNode); | ||
| System.out.println(validJsonNode.toPrettyString()); |
There was a problem hiding this comment.
Please remove print line and duplicate blank line.
| @Test | ||
| public void wrongProhibition_testContractPolicyConstraints_fails() throws JsonProcessingException { | ||
| // given | ||
| String validJson = "{\n" + |
There was a problem hiding this comment.
Checked for other method: let's make the validJson and invalidJson.
| * @throws JsonProcessingException if json is invalid | ||
| */ | ||
| @Test | ||
| public void wrongProhibition_testContractPolicyConstraints_fails() throws JsonProcessingException { |
There was a problem hiding this comment.
I was wondering if it makes sense to summarize the two tests into one parametrized test like in mad for the pattern checks.
Then I would propose to make the jsons constants with names like CATALOG_PROHIBITION_SET_NOT_ALLOWED and a comment. That would keep some readability.
Being honest: I also like the test name stating directly wrongProhibition and wrongObligation directly in the beginning. So this is not a neccessary change but an idea - only adapt if you like it better, because I'm not sure what's easier to maintain :)
tom-rm-meyer-ISST
left a comment
There was a problem hiding this comment.
LGTM, thanks a lot!
Description
Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review: