[
https://issues.apache.org/jira/browse/HADOOP-18866?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17760943#comment-17760943
]
ASF GitHub Bot commented on HADOOP-18866:
-----------------------------------------
steveloughran commented on PR #5982:
URL: https://github.com/apache/hadoop/pull/5982#issuecomment-1701070887
i do think test age is an issue, and old code is suboptimal, but using
test(expected) over our own intercept() or even assertThrows isn't something
we'd do today.
now, serious question for you: if a test is working, why does it need it
maintenance?
because there are other uses of engineering time, including
* new features
* bug fixes in production
* writing new tests
* reviewing patches by others
*
so, how would you justify the change, as "it's considered old fashioned"
doesn't do it.
if anyone was to go near old tests, i'd target things i really don't like
1. assertTrue/assertFalse without error messages or detail why the test
failed. Fix: use assertJ assertions with .describedAs(), or at least add
messages
2. assertEquals with the arguments reversed. fix, swap, or better: assertj.
3. code which uses try/catch/fail rather than intercept(). sadly too common
moving #3 onto intercept() would be better than worrying about "expected";
while #1 is critical for debugging a test report "testXYZ failed on Line 222".
It's why I automatically veto any PR where new tests don't add details. Still
disappointed that people still submit PRs where they don't do that. very bad
style. we need a stylechecker rule for it, at least for new code,
> Refactor @Test(expected) with assertThrows
> ------------------------------------------
>
> Key: HADOOP-18866
> URL: https://issues.apache.org/jira/browse/HADOOP-18866
> Project: Hadoop Common
> Issue Type: Improvement
> Reporter: Taher Ghaleb
> Priority: Minor
> Labels: pull-request-available
>
> I am working on research that investigates test smell refactoring in which we
> identify alternative implementations of test cases, study how commonly used
> these refactorings are, and assess how acceptable they are in practice.
> The smell occurs when exception handling can alternatively be implemented
> using assertion rather than annotation: using {{assertThrows(Exception.class,
> () -> \{...});}} instead of {{{}@Test(expected = Exception.class){}}}.
> While there are many cases like this, we aim in this pull request to get your
> feedback on this particular test smell and its refactoring. Thanks in advance
> for your input.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
