adoroszlai commented on code in PR #8034:
URL: https://github.com/apache/ozone/pull/8034#discussion_r1986284417
##########
hadoop-hdds/client/dev-support/findbugsExcludeFile.xml:
##########
@@ -19,4 +19,16 @@
<Class name="org.apache.hadoop.hdds.scm.storage.ByteArrayReader"></Class>
<Bug pattern="EI_EXPOSE_REP2" /> <!-- "Deep copy byte[] has bad impact on
performance" -->
</Match>
+ <Match>
+ <Class
name="org.apache.hadoop.hdds.scm.storage.DummyBlockInputStreamWithRetry"/>
+ <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
+ </Match>
+ <Match>
+ <Class name="org.apache.hadoop.hdds.scm.storage.TestBlockInputStream"/>
+ <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
+ </Match>
+ <Match>
+ <Class
name="org.apache.hadoop.ozone.client.io.TestBlockInputStreamFactoryImpl"/>
+ <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
+ </Match>
Review Comment:
Please do not add these.
- It is completely non-scalable to add an exclusion for each test class.
- We should reduce the number of exclusions, not increase it (HDDS-12342).
- Only 3 classes are added here, but 5 classes are changed similarly. I'm
guessing the others were not reported by spotbugs. The problem is that
spotbugs analysis is not 100% accurate, and sometimes starts reporting existing
problems only later, when making changes in other classes.
This is a spotbugs problem. @peterxcli reported it few weeks ago:
https://github.com/apache/ozone/pull/7963#discussion_r1968652018
I don't know if the problem is fixed by more recent versions of spotbugs.
Unfortunately we have to use an older version of spotbugs due to lots of new
issues reported by newer one (HDDS-10150).
I think we should either
- defer mass conversion to `doReturn` until we have a version of spotbugs
where this is fixed, OR
- add exclusion for this issue in all test classes using pattern matching,
AND enable similar rule in PMD (subtask of HDDS-12338).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
