[
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: [email protected]
For additional commands, e-mail: [email protected]
