[GitHub] [james-project] chibenwa commented on a change in pull request #288: James 3492

[GitBox <gi...@apache.org>]

chibenwa commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r570974375



##########
File path: backends-common/elasticsearch-v7/src/test/java/org/apache/james/backends/es/v7/ElasticSearchHealthCheckTest.java
##########
@@ -68,7 +68,7 @@ private FakeClusterHealthResponse(ClusterHealthStatus clusterHealthStatus) {
             super("fake-cluster", new String[0],

Review comment:
       I don't understand this commit message: `JAMES-3492, repaijava errors in backend module tests `... Is there a typo? Can you reword it?

##########
File path: mailbox/elasticsearch-v7/src/test/java/org/apache/james/mailbox/elasticsearch/v7/json/MessageToElasticSearchJsonTest.java
##########
@@ -132,6 +133,7 @@ void invalidCharsetShouldBeWellConvertedToJson() throws IOException {
     }
 
     @Test
+    @Disabled

Review comment:
       I'm curious to know why these tests are failing as they do not directly depends on ElasticSearch but as far as I know on Jackson...

##########
File path: mailbox/elasticsearch-v7/src/test/java/org/apache/james/mailbox/elasticsearch/v7/ElasticSearchIntegrationTest.java
##########
@@ -129,6 +128,7 @@ protected void initializeMailboxManager() throws Exception {
     }
 
     @Test
+	@Disabled("Types cannot be provided in get mapping requests, unless include_type_name is set to true.")

Review comment:
       We tend to follow the convention to put @Test and @Disabled at the same indentation level. Any reason to be doing some different?

##########
File path: backends-common/elasticsearch-v7/src/test/java/org/apache/james/backends/es/v7/ElasticSearchIndexerTest.java
##########
@@ -38,6 +38,7 @@
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;

Review comment:
       Any hint why these tests are failing?

##########
File path: server/container/metrics/metrics-es-reporter-v7/src/test/java/org/apache/james/metric/es/v7/ES7ReporterTest.java
##########
@@ -20,9 +20,11 @@
 package org.apache.james.metric.es.v7;
 
 import org.apache.james.backends.es.v7.DockerElasticSearch;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-class ES7ReporterTest extends ESReporterContract {
+@Disabled
+class ES7ReporterTest { //TODO extends ESReporterContract {

Review comment:
       Why moving this extends as a TODO that is so easy to forget?

##########
File path: mailbox/elasticsearch-v7/src/test/java/org/apache/james/mailbox/elasticsearch/v7/ElasticSearchIntegrationTest.java
##########
@@ -339,6 +348,7 @@ void localPartShouldBeMatchedWhenHyphen() throws Exception {
 
     @Disabled("MAILBOX-401 '-' causes address matching to fail")
     @Test
+	//@Disabled("Types cannot be provided in get mapping requests, unless include_type_name is set to true.")

Review comment:
       Do we need these commented `@Disabled`?

##########
File path: mailbox/plugin/quota-search-elasticsearch-v7/src/test/java/org/apache/james/quota/search/elasticsearch/v7/ElasticSearchQuotaSearcherTest.java
##########
@@ -26,15 +26,17 @@
 
 import org.apache.james.core.Username;
 import org.apache.james.core.quota.QuotaSizeLimit;
-import org.apache.james.quota.search.Limit;
-import org.apache.james.quota.search.Offset;
-import org.apache.james.quota.search.QuotaQuery;
-import org.apache.james.quota.search.QuotaSearchTestSystem;
-import org.apache.james.quota.search.QuotaSearcherContract;
+import org.apache.james.domainlist.api.DomainListException;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.quota.search.*;

Review comment:
       We avoid wildcard imports. Maybe you should check and adjust your IDE import settings.
   
   (checkstyle sadly do not run on test classes.)




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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org