[
https://issues.apache.org/jira/browse/YARN-4927?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15231351#comment-15231351
]
Karthik Kambatla commented on YARN-4927:
----------------------------------------
Thanks for picking this up, [~bibinchundatt].
Few comments:
# AdminService: since the test is in the same class, {{refreshAll}} could be
package-private instead of public. Also, you might want to mark it
@VisibleForTesting along with a comment that it can be private otherwise.
# TestRMHA
## The new variable counter could be private to the anonymous AdminService
class we are creating in the test.
## The assertion when the RM fails to transition to active seems backwards.
Shouldn't we be checking {{e.getMessage().contains("<blah>")}}?
## I wonder if we are even running into that exception. If the test is
expecting the exception, we should add an {{Assert.fail}} right after the call
to transition to active.
## Also, I am not a fan of checking just the message verbatim. Can we check if
the exception is {{ServiceFailedException}} and preferably the expected RM
state (Active/Standby)?
## Not introduced in this patch, but the asserts in the test should have a
corresponding error message to explain what exactly is going on .
> TestRMHA#testTransitionedToActiveRefreshFail fails when FairScheduler is the
> default
> ------------------------------------------------------------------------------------
>
> Key: YARN-4927
> URL: https://issues.apache.org/jira/browse/YARN-4927
> Project: Hadoop YARN
> Issue Type: Bug
> Components: test
> Affects Versions: 2.8.0
> Reporter: Karthik Kambatla
> Assignee: Bibin A Chundatt
> Attachments: 0001-YARN-4927.patch
>
>
> YARN-3893 adds this test, that relies on some CapacityScheduler-specific
> stuff for refreshAll to fail, which doesn't apply when using FairScheduler.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
