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

Reply via email to