[
https://issues.apache.org/jira/browse/YARN-9476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16824137#comment-16824137
]
Szilard Nemeth commented on YARN-9476:
--------------------------------------
Hi [~pbacsko]!
Thanks for this patch!
Some comments:
1. The line with field {{commandExecutorProvider}} doesn't need a line break,
can fit in a single line.
2. In setup: You can use "expression lambdas" for envProvider /
commandExecutorProvider.
E.g.
{code:java}
envProvider = (String var) -> env.get(var);
{code}
3. In general, I prefer to move private methods to the top of the class, but it
can be a taste decision.
Anyway, while I was reading the code of the setup method, I was trying to find
where is the getOutputForDevice method. Could you move it to the top of the
class (after setup)?
4. Line
{code:java}
Comparator<Device> cmp = (Device d1, Device d2) -> d1.getId() - d2.getId();
{code}
can be replaced with
{code:java}
Comparator<Device> cmp = Comparator.comparingInt(Device::getId);
{code}
As you are using this comparator many times, please extract it to a field.
5. Some comments are unnecessary, like:
{code:java}
// create new directory
// create new
// create script
{code}
6. Instead of deleting the test folder in finally blocks, you should put it in
a JUnit-after method so that if the test dir exists (you can refer to it as it
is already a field), it will be removed.
7. You have some tests where you set the executable flag of a file: Please have
asserts on the results, as it can happen that in some circumstances, the
operation won't be successful.
8. Similarly to 7., when you call mkdirs() on a File object, you should assert
the result if it's successful.
Thanks!
> Create unit tests for VE plugin
> -------------------------------
>
> Key: YARN-9476
> URL: https://issues.apache.org/jira/browse/YARN-9476
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Peter Bacsko
> Assignee: Peter Bacsko
> Priority: Major
> Attachments: YARN-9476-001.patch, YARN-9476-002.patch
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
