[
https://issues.apache.org/jira/browse/YARN-9123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16765911#comment-16765911
]
Adam Antal commented on YARN-9123:
----------------------------------
Hi [~snemeth]. Thanks for the patch! The cleaning looks good, I have some minor
comments to that.
- The following piece of code is replicated, as you have split the test into 3
parts:
{code:java}
assertEquals("MediaType of the response is not the expected!",
MediaType.APPLICATION_JSON + "; " + JettyUtils.UTF_8,
response.getType().toString());
json = response.getEntity(JSONObject.class);
Assert.assertEquals(1000, json.get("a"));
JSONObject json = response.getEntity(JSONObject.class);
assertEquals("Unexpected value in the json response!",
0, json.length());
{code}
Consider putting it to a separate function, and calling that in order to avoid
minor code duplication.
- Though I'm not an expert on this area, it seems strange that there is no
logging at all (does not even exist a logging class). Though it would go beyond
the scope of this jira, I recommend inserting a log object and a few debug
logging: like successfully setting up mocks, respond successfully received.
(Only for the tests you marked for refactoring).
- The Test testGetNMResourceInfoFailBecauseOfUnknownPlugin is a bit lengthy:
47 character. Though it's completely allowed, we can name it to a bit shorter
one for better readability. There aren't any javadocs in the nearby testcases,
but you can also move pieces of information about the test to javadoc.
> Clean up and split testcases in TestNMWebServices for GPU support
> -----------------------------------------------------------------
>
> Key: YARN-9123
> URL: https://issues.apache.org/jira/browse/YARN-9123
> Project: Hadoop YARN
> Issue Type: Improvement
> Reporter: Szilard Nemeth
> Assignee: Szilard Nemeth
> Priority: Minor
> Attachments: YARN-9123.001.patch, YARN-9123.002.patch,
> YARN-9123.003.patch, YARN-9123.004.patch
>
>
> The following testcases can be cleaned up a bit:
> TestNMWebServices#testGetNMResourceInfo - Can be split up to 3 different cases
> TestNMWebServices#testGetYarnGpuResourceInfo
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
