[jira] [Commented] (YARN-8750) Refactor TestQueueMetrics

["Szilard Nemeth (JIRA)" <ji...@apache.org>]

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