I agree that having to write "MatcherAssert.assertThat" each time is tedious and makes my code ugly. So finally I try to avoid this nice construction. Not satisfying.
Le mer. 25 oct. 2023 à 22:44, Kevin Risden <kris...@apache.org> a écrit : > It came in as part of > https://github.com/apache/solr/pull/947#issuecomment-1279651282 > > I linked to one of the comments there since this was discussed on the PR. > Basically it removes a ton of deprecated usages. If/when we upgrade Junit > we should hopefully have much less to do. > > So to answer your question > > > - What is so terrible about the impl of Assert.assertThat? > > > > Nothing just that it is deprecated and trying to stay on top of avoiding > the use of deprecated code if we can. > > - What evil does it contain that makes it completely unssuitable for use? > > > > Again same as above since it is the same question twice. > > I'll give you that its not ideal, but we have to draw the line somewhere. > > Kevin Risden > > > On Wed, Oct 25, 2023 at 4:27 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 > > > > >
