You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2021/02/20 07:19:00 UTC

[GitHub] [logging-log4j2] rgoers commented on a change in pull request #468: Apply automated Assertj refactors to core modules

rgoers commented on a change in pull request #468:
URL: https://github.com/apache/logging-log4j2/pull/468#discussion_r579613637



##########
File path: log4j-1.2-api/src/test/java/org/apache/log4j/CallerInformationTest.java
##########
@@ -57,9 +57,9 @@ public void testMethodLogger() throws Exception {
         logger.warn("brains~~~");
         logger.info("Itchy. Tasty.");
         final List<String> messages = app.getMessages();
-        assertEquals("Incorrect number of messages.", 5, messages.size());
+        assertThat(messages).describedAs("Incorrect number of messages.").hasSize(5);

Review comment:
       Not that I object to these but I am not sure why the new version is "better".  In this case they both effectively say "Throw an exception with this message if the message list doesn't contain 5 items and to me both are equally clear.

##########
File path: log4j-1.2-api/src/test/java/org/apache/log4j/CategoryTest.java
##########
@@ -78,23 +78,23 @@ public void testForcedLog() {
         category.info("Hello, World");
         final List<LogEvent> list = appender.getEvents();
         int events = list.size();
-        assertTrue("Number of events should be 1, was " + events, events == 1);
+        assertThat(events).describedAs("Number of events should be 1, was " + events).isEqualTo(1);
         LogEvent event = list.get(0);
         Message msg = event.getMessage();
-        assertNotNull("No message", msg);
-        assertTrue("Incorrect Message type", msg instanceof ObjectMessage);
+        assertThat(msg).describedAs("No message").isNotNull();
+        assertThat(msg instanceof ObjectMessage).describedAs("Incorrect Message type").isTrue();
         Object[] objects = msg.getParameters();
-        assertTrue("Incorrect Object type", objects[0] instanceof String);
+        assertThat(objects[0] instanceof String).describedAs("Incorrect Object type").isTrue();
         appender.clear();
         category.log(Priority.INFO, "Hello, World");
         events = list.size();
-        assertTrue("Number of events should be 1, was " + events, events == 1);
+        assertThat(events).describedAs("Number of events should be 1, was " + events).isEqualTo(1);
         event = list.get(0);
         msg = event.getMessage();
-        assertNotNull("No message", msg);
-        assertTrue("Incorrect Message type", msg instanceof ObjectMessage);
+        assertThat(msg).describedAs("No message").isNotNull();
+        assertThat(msg instanceof ObjectMessage).describedAs("Incorrect Message type").isTrue();

Review comment:
       I do notice a pattern. Every one of the replacements is longer than the original. Of course, with IDE code completion that doesn't necessarily imply more typing.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org