You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/05/05 09:47:37 UTC

[GitHub] [kafka] cadonna commented on a change in pull request #10631: MINOR: Stop using hamcrest in system tests

cadonna commented on a change in pull request #10631:
URL: https://github.com/apache/kafka/pull/10631#discussion_r626415437



##########
File path: streams/src/test/java/org/apache/kafka/streams/tests/RelationalSmokeTest.java
##########
@@ -833,33 +820,47 @@ static boolean verifySync(final boolean logResults,
             final AtomicBoolean pass = new AtomicBoolean(true);
             final StringBuilder report = logResults ? new StringBuilder() : null;
 
-            assertThat(pass, report, "one article", consumedArticles.size(), greaterThan(0));
-            assertThat(pass, report, "one comment", consumedComments.size(), greaterThan(0));
+            assertThat(
+                pass,
+                report,
+                "Expected 1 article, got " + consumedArticles.size(),
+                consumedArticles.size() > 0
+            );
+            assertThat(
+                pass,
+                report,
+                "Expected 1 comment, got " + consumedComments.size(),
+                consumedComments.size() > 0
+            );
 
             assertThat(
                 pass,
                 report,
-                "article size",
-                consumedAugmentedArticles.size(),
-                is(consumedArticles.size())
+                "Mismatched article size between augmented articles (size "
+                        + consumedAugmentedArticles.size() +
+                        ") and consumed articles (size "
+                        + consumedArticles.size() + ")",
+                consumedAugmentedArticles.size() == consumedArticles.size()
             );
             assertThat(
                 pass,
                 report,
-                "comment size",
-                consumedAugmentedComments.size(),
-                is(consumedComments.size())
+                "Mismatched comments size between augmented comments (size "
+                        + consumedAugmentedComments.size() +
+                        ") and consumed comments (size " +
+                        consumedComments.size() + ")",
+                consumedAugmentedComments.size() == consumedComments.size()
             );
 
             final Map<Integer, Long> commentCounts = new TreeMap<>();
 
             for (final RelationalSmokeTest.AugmentedComment augmentedComment : consumedAugmentedComments.values()) {
                 final int key = augmentedComment.getKey();
                 assertThat(pass,
-                           report,
-                           "comment missing, but found in augmentedComment: " + key,
-                           consumedComments,
-                           hasKey(key));
+                        report,
+                        "comment missing, but found in augmentedComment: " + key,
+                        consumedComments.containsKey(key)
+                );

Review comment:
       nit: I think that should be formatted like:
   
   ```
   assertThat(
       pass,
       report,
       "comment missing, but found in augmentedComment: " + key,
       consumedComments.containsKey(key)
   );
   ```

##########
File path: streams/src/test/java/org/apache/kafka/streams/tests/RelationalSmokeTest.java
##########
@@ -868,47 +869,43 @@ static boolean verifySync(final boolean logResults,
                         report,
                         "comment articleId [" + comment.getArticleId() + "] didn't match " +
                             "augmentedComment articleId [" + augmentedComment.getArticleId() + "]",
-                        comment.getArticleId(),
-                        is(augmentedComment.getArticleId())
+                        comment.getArticleId() == augmentedComment.getArticleId()
                     );
                 }
                 commentCounts.put(augmentedComment.getArticleId(),
-                                  commentCounts.getOrDefault(augmentedComment.getArticleId(), 0L) + 1);
+                                      commentCounts.getOrDefault(augmentedComment.getArticleId(), 0L) + 1);

Review comment:
       nit: formatting
   
   ```
   commentCounts.put(
       augmentedComment.getArticleId(),
       commentCounts.getOrDefault(augmentedComment.getArticleId(), 0L) + 1
   );
   ```

##########
File path: streams/src/test/java/org/apache/kafka/streams/tests/RelationalSmokeTest.java
##########
@@ -919,8 +916,8 @@ static boolean verifySync(final boolean logResults,
                     pass,
                     report,
                     "article " + augmentedArticle.getKey() + " comment count mismatch",
-                    augmentedArticle.getCommentCount(),
-                    is(commentCounts.getOrDefault(augmentedArticle.getKey(), 0L))
+                    augmentedArticle.getCommentCount()
+                            == commentCounts.getOrDefault(augmentedArticle.getKey(), 0L)

Review comment:
       nit:
   
   ```suggestion
                       augmentedArticle.getCommentCount() == commentCounts.getOrDefault(augmentedArticle.getKey(), 0L)
   ```

##########
File path: streams/src/test/java/org/apache/kafka/streams/tests/RelationalSmokeTest.java
##########
@@ -833,33 +820,47 @@ static boolean verifySync(final boolean logResults,
             final AtomicBoolean pass = new AtomicBoolean(true);
             final StringBuilder report = logResults ? new StringBuilder() : null;
 
-            assertThat(pass, report, "one article", consumedArticles.size(), greaterThan(0));
-            assertThat(pass, report, "one comment", consumedComments.size(), greaterThan(0));
+            assertThat(
+                pass,
+                report,
+                "Expected 1 article, got " + consumedArticles.size(),
+                consumedArticles.size() > 0
+            );
+            assertThat(
+                pass,
+                report,
+                "Expected 1 comment, got " + consumedComments.size(),
+                consumedComments.size() > 0
+            );
 
             assertThat(
                 pass,
                 report,
-                "article size",
-                consumedAugmentedArticles.size(),
-                is(consumedArticles.size())
+                "Mismatched article size between augmented articles (size "
+                        + consumedAugmentedArticles.size() +
+                        ") and consumed articles (size "
+                        + consumedArticles.size() + ")",

Review comment:
       nit:
   ```suggestion
                   "Mismatched article size between augmented articles (size "
                       + consumedAugmentedArticles.size() +
                       ") and consumed articles (size "
                       + consumedArticles.size() + ")",
   ```




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