strangelookingnerd commented on PR #470: URL: https://github.com/apache/httpcomponents-core/pull/470#issuecomment-2191634860
> > Make sure tests have at least one assertion > > I am aware of this recommendation but I always felt it was not justified / too extreme. I myself am on the edge for this. I do agree that there may be some exceptions, where the goal of a test is given by its name and it does not ultimately need an assertion. However when looking at tests like these: ```java @Test void testConstructor() { new MyObject(); } ``` I personally feel that this can be improved by something like this in order to make it clear to the reader what is intended to be tested: ```java @Test void testConstructor() { Assertions.assertDoesNotThrow(() -> new MyObject()); } ``` This is a personal preference, however. > > Is there a chance you could split this PR into multiple commits? Can we start with ` Test classes and methods should have default package visibility`, `Remove duplicate tests` and `Fix assertion argument orders`? I can defintitly try it, I will force-push the changes once I'm done and ping you 👍🏼 -- 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: dev-unsubscr...@hc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org