[ https://issues.apache.org/jira/browse/YARN-8750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16634691#comment-16634691 ]
Szilard Nemeth commented on YARN-8750: -------------------------------------- The changes in TestQueueMetrics could have been more simple if I had used a map, but using a separate "checker" class for verification is having some advantages that are not visible in the first place: 1. The possiblity of accidentally interchange Resource metrics and App metrics assertions are enforced, there are dedicated checker classes for those along with their respective enums. For example, {{AppMetricsChecker}} only accepts {{AppMetricsKey}}s. The same goes with {{ResourceMetricsChecker}} and {{ResourceMetricsKey}}s. 2. The mentioned enums guarantee that only existing resource metrics / app metrics keys are used in tests. 3. The methods named as {{checkAll}} in the two checker classes are hiding the complexity of asserting gauge and counter values. As the functionality of {{checkAll}} could be replaced with 3 maps in every test classes where the test want to verify metrics, this could lead to unnecessary code duplication, so the current solution is more reusable. 4. Methods {{gaugeLong}}, {{gaugeInt}} and {{counter}} in {{ResourceMetricsChecker}} put the values in the correct map. If the tests themselves were referencing those maps, it would be easier to put the value to a wrong map unintentionally. I'm open to rename the {{checkAll}} method as one can come up with a better name, but that's what I got for now. > Refactor TestQueueMetrics > ------------------------- > > Key: YARN-8750 > URL: https://issues.apache.org/jira/browse/YARN-8750 > Project: Hadoop YARN > Issue Type: Improvement > Components: resourcemanager > Reporter: Szilard Nemeth > Assignee: Szilard Nemeth > Priority: Minor > Attachments: YARN-8750.001.patch, YARN-8750.002.patch, > YARN-8750.003.patch > > > {{TestQueueMetrics#checkApps}} and {{TestQueueMetrics#checkResources}} have 8 > and 14 parameters, respectively. > It is very hard to read the testcases that are using these methods. -- 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