YARN-11172. Fix TestClientRMTokens#testDelegationToken introduced by HDFS-16563.#4408
YARN-11172. Fix TestClientRMTokens#testDelegationToken introduced by HDFS-16563.#4408steveloughran merged 2 commits intoapache:trunkfrom
Conversation
|
🎊 +1 overall
This message was automatically generated. |
|
But it is recommended to write the title clearly. use has expired may be better and remove extra spaces. |
|
@Hexiaoqiao @steveloughran Can you please review the PR? Some unit test fail after HDFS-16563. |
|
LambdaTestUtils.intercept is the way to test this; its our reimplementation of ScalaTest intercept. if the exception class or message is wrong, the exception is rethrown. which would you prefer in jenkins log. a message saying "an assert true failed" or "here is the exception which was raised with all its stack"? |
Fail log are: Exception message changed by HDFS-16563. But this unit test just check the message. |
I will fix the code like Should I fix all code in this style? Because I found many code in TestClientRMTokens need to fix in this way. |
|
just do the one which is broken. yes, the rest is out of date, but it would make for a larger patch, harder to cherrypick into branch-3.3 etc. with intercept() if the wrong exception type or message is found, the exception is thrown again, so we understand what is there. |
There was a problem hiding this comment.
this is pretty old code; we would never write new tests this way.
as well as being complicated, if a different exception is raised we want that stack trace
best to clean up
replace the whole section from L202 with a call of
LambdaTestUtils.intercept(InvalidToken.class, "has expired",
() -> clientRMWithDT.getNewApplication(request));look inside the intercept call to see what it does better.
There was a problem hiding this comment.
I have fix this code according to your advice. Maybe it's outdated code, I have rebase it.
...emanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMTokens.java
Show resolved
Hide resolved
|
@steveloughran Can you please review it again and merge to trunk? Some PR is blocked by this. |
…DFS-16563. (#4408) Regression caused by HDFS-16563; the hdfs exception text was changed, but because it was a YARN test doing the check, Yetus didn't notice. Contributed by zhengchenyu
…HDFS-16563. (apache#4408) Regression caused by HDFS-16563; the hdfs exception text was changed, but because it was a YARN test doing the check, Yetus didn't notice. Contributed by zhengchenyu
Description of PR
UT fail after [HDFS-16563], other yarn PR is blocked.