[ 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: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org