This would be cleared up if we change LuceneTestCase? ~ David
On Wed, Oct 25, 2023 at 4:26 PM Chris Hostetter <hossman_luc...@fucit.org> wrote: > > Begin Rant... > > $ tail -6 ./gradle/validation/forbidden-apis/junit.junit.all.txt > junit.framework.TestCase @ All classes should derive from LuceneTestCase > junit.framework.Assert @ Use org.junit.Assert > junit.framework.** @ Use org.junit > org.junit.Assert#assertThat(**) @ Use > org.hamcrest.MatcherAssert.assertThat() > org.junit.matchers.JUnitMatchers @ Use org.hamcrest.CoreMatchers > org.junit.rules.ExpectedException @ Use Assert.assertThrows > > So all test classes should: > - derive from LuceneTestCase (agreed) > - use org.junit.Assert instead of junit.framework.Assert (fair enough) > - use MatcherAssert.assertThat() instead of Assert.assertThat() ... > > ...ok, i guess? ... except that LuceneTestCase extends Assert, giving us > Assert.assertThat() for free. Kind of anoying to have to go out of our > way to import another class to do the same thing. > > But also ... you can't just use... > > import static org.hamcrest.MatcherAssert.assertThat; > > ...because (assuming you're following best practices) you're already > inheriting from LuceneTestCase, so that static import is going to be > unused since it conflicts with the inherited 'assertThat' -- or at least > that's what ecjLintTest tells you ... i'm not 100% certain it's correct, > but either way you can't use it because it will fails the build. > > So your only recourse is to use... > > import org.hamcrest.MatcherAssert; > > ...and now instead of lots of nice short 'assertThat(...)' method calls > you have to use the twice as long 'MatcherAssert.assertThat', which is > probably going to make spotless decide your assertions needs to span at > least one extra line (assuming you are using descriptive variable names) > > All because we don't want want devs to use a (deprecated) method > that's indirectly inherited from an ancestor of a baseclass we tell them > to use. > > Which begs the questions: > > - What is so terrible about the impl of Assert.assertThat? > - What evil does it contain that makes it completely unssuitable for use? > > Let's go find out what exactly we are asking forbidden-apis to > shield us from... > > > https://github.com/junit-team/junit4/blob/r4.13.2/src/main/java/org/junit/Assert.java#L962 > > public static <T> void assertThat(String reason, T actual, > Matcher<? super T> matcher) { > MatcherAssert.assertThat(reason, actual, matcher); > } > > ...oh dear lord ... what sweet horror is this??!?!?!!? > > Thank heavens we were wise enough to protect ourselves from this > unspeakably dangerous code! > > > > -Hoss > http://www.lucidworks.com/ > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@solr.apache.org > For additional commands, e-mail: dev-h...@solr.apache.org > >
