[jira] [Commented] (YARN-9362) Code cleanup in TestNMLeveldbStateStoreService

["Peter Bacsko (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/YARN-9362?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16967545#comment-16967545 ] 

Peter Bacsko commented on YARN-9362:
------------------------------------

Thanks for the patch [~denes.gerencser]. I don't have too much context, but having smaller tests instead of a big one is certainly much better. I just have two comments right now:

1) Would be good to see some nice assertion messages. However, it's missing everywhere and I don't think it's worth the hassle, so let's keep it that way.
2) I'm wondering if using {{System.currentTimeMillis()}} is necessary:
{noformat}
long containerStartTime = System.currentTimeMillis();
{noformat}

can't we just use a constant?



> Code cleanup in TestNMLeveldbStateStoreService
> ----------------------------------------------
>
>                 Key: YARN-9362
>                 URL: https://issues.apache.org/jira/browse/YARN-9362
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Szilard Nemeth
>            Assignee: Denes Gerencser
>            Priority: Minor
>         Attachments: YARN-9362.001.patch
>
>
> There are many ways to improve TestNMLeveldbStateStoreService: 
> 1. RecoveredContainerState fields are asserted many times repeatedly. Some simple method extractions would definitely make this more readable.
> 2. The tests are very long and hard to read in general: Again, finding how methods could be extracted to avoid code repetition could help. 
> 3. You name it.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org