You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/02/05 13:29:02 UTC

[GitHub] [james-project] andrevka opened a new pull request #288: James 3492

andrevka opened a new pull request #288:
URL: https://github.com/apache/james-project/pull/288


   Duplicate elasticsearch related modules, and use 7.10.2 clients and docker images in those modules.


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


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

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571855787



##########
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:
       https://github.com/apache/james-project/pull/288/commits/16c4df23588581471bc96b3cc6be732e6d12cfde




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


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

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571855040



##########
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:
       They failed previously, something wrong with my environment. I removed the @Disabled and the tests passed.




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


[GitHub] [james-project] Arsnael closed pull request #288: James 3492

Posted by GitBox <gi...@apache.org>.
Arsnael closed pull request #288:
URL: https://github.com/apache/james-project/pull/288


   


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


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

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571860830



##########
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:
       https://github.com/apache/james-project/pull/288/commits/a65445a5a67bd41124021b207ef4ae8d2067540b, seems there were no more classes with this issue
   




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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #288:
URL: https://github.com/apache/james-project/pull/288#issuecomment-775663916


   You can cherry-pick https://github.com/linagora/james-project/pull/4258/commits/c5dc62ca3b4d66587307092938443063774ab9ca which eventually fixes compilation...


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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #288:
URL: https://github.com/apache/james-project/pull/288#issuecomment-781217258


   Hey there!
   
   We did a quick MOB programming around this topic with Rene and Lan. We did play around `IndexerTest` trying to re-enable them.
   
   We noticed the `regular INDEX + flush` is no longer enough to get indexed data exposed to search, and was causing tests to crash.
   
   After trying without success several variations of the FLUSH command, we ended up trying:
    - Synchronous INDEX. The indexer specifies parameters for waiting the INDEXed document  exposed to search before returning. This, is of course NOT production friendly (where you likely want to delay refreshes for minutes) and do not want thousands of indexes pending. However we could specify this option only in tests and get rid of all the `await elasticsearch`.
    - Wrapping all our assertions within `CALMY_AWAIT.untilAsserted`, wich demands to revisit the entire testing logic.
    
    We opted for the second to minimize the distance between our testing set up and production environment, the downside being that it leads to a large refactoring (calmly awaiting in a good portion of the code base) and potentially to unstable tests if an await statement is missing.
    
    Note that the awaits will thus no longer be performed at the DB level but likely linked to the testing logic.
    
    Here is the output of our MOB session: https://github.com/apache/james-project/pull/302/commits/e2d06655b66a0cee777952bd9fd3610c9cd70241


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


[GitHub] [james-project] Arsnael commented on pull request #288: James 3492

Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #288:
URL: https://github.com/apache/james-project/pull/288#issuecomment-788578825


   Has been merged, but still need a bit more work. Thanks for the work @andrevka :)


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


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

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571850128



##########
File path: mailbox/elasticsearch-v7/src/test/java/org/apache/james/mailbox/elasticsearch/v7/AbstractMessageSearchIndexTest.java
##########
@@ -0,0 +1,1599 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ m10 = otherInboxMessageManager.appendMessage(
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailbox.elasticsearch.v7;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.*;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.model.*;
+import org.apache.james.mailbox.model.SearchQuery.AddressType;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.mailbox.model.SearchQuery.Sort;
+import org.apache.james.mailbox.model.SearchQuery.Sort.Order;
+import org.apache.james.mailbox.model.SearchQuery.Sort.SortClause;
+import org.apache.james.mailbox.store.StoreMailboxManager;
+import org.apache.james.mailbox.store.StoreMessageManager;
+import org.apache.james.mailbox.store.search.MessageSearchIndex;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.dom.Multipart;
+import org.apache.james.mime4j.message.BodyPartBuilder;
+import org.apache.james.mime4j.message.MultipartBuilder;
+import org.apache.james.mime4j.message.SingleBodyBuilder;
+import org.apache.james.util.ClassLoaderUtils;
+import org.awaitility.Awaitility;
+import org.awaitility.Duration;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Flux;
+
+import javax.mail.Flags;
+import java.nio.charset.StandardCharsets;
+import java.text.SimpleDateFormat;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.List;
+import java.util.TimeZone;
+import java.util.concurrent.TimeUnit;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+@Disabled
+public abstract class AbstractMessageSearchIndexTest {

Review comment:
       https://github.com/apache/james-project/pull/288/commits/158fff73ba93bc7b7bb2f3681cb8f813549d6872




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


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

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571850379



##########
File path: backends-common/elasticsearch-v7/src/test/java/org/apache/james/backends/es/v7/ElasticSearchIndexerTest.java
##########
@@ -80,6 +81,7 @@ void tearDown() throws IOException {
     }
 
     @Test
+    @Disabled

Review comment:
       Added ticket number to Disabled tags




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


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

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571860830



##########
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:
       https://github.com/apache/james-project/pull/288/commits/a65445a5a67bd41124021b207ef4ae8d2067540b, seems there were no more classes with this issu




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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #288:
URL: https://github.com/apache/james-project/pull/288#issuecomment-775770297


   https://github.com/linagora/james-project/pull/4258/commits/bbce07fe31327c5af3e64735e17e2ac31bbaf7ad did rebase this on master and fixed compile issues


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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r570975235



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




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


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

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571850800



##########
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:
       Removed in https://github.com/apache/james-project/pull/288/commits/467f6cbc9c2eeba393ebf611d5ba0c466ac8aaf3




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


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

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571807539



##########
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:
       Changed commit message _JAMES-3492, repair code errors after client library change in backend es module tests_




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


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

Posted by GitBox <gi...@apache.org>.
andrevka commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571851513



##########
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:
       Removed in https://github.com/apache/james-project/pull/288/commits/81ff164a5b6ee7afdbb2c1dfdfbd340e4cb88c38




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


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

Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r577314046



##########
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.*;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
 @ExtendWith(ElasticSearchQuotaSearchTestSystemExtension.class)
+@Disabled

Review comment:
       As @mbaechler said, here it's the same I believe, tests should be disabled, not the test class

##########
File path: server/container/metrics/metrics-es-reporter-v7/src/test/java/org/apache/james/metric/es/v7/ES6ReporterTest.java
##########
@@ -20,10 +20,12 @@
 package org.apache.james.metric.es.v7;
 
 import org.apache.james.backends.es.v7.DockerElasticSearch;
+import org.apache.james.util.docker.Images;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 class ES6ReporterTest extends ESReporterContract {

Review comment:
       Throwing a question here: do we really want to keep `ES2ReporterTest` and `ES6ReporterTest` here? What pushed us to keep `ES2ReporterTest` here in the first place? I forgot

##########
File path: mailbox/elasticsearch-v7/src/test/java/org/apache/james/mailbox/elasticsearch/v7/ElasticSearchIntegrationTest.java
##########
@@ -81,12 +83,14 @@ void tearDown() throws IOException {
         client.close();
     }
 
-    @Override
+    @Disabled("JAMES-3492")

Review comment:
       This is not a test, no need to disable it

##########
File path: mailbox/elasticsearch-v7/src/test/java/org/apache/james/mailbox/elasticsearch/v7/ElasticSearchIntegrationTest.java
##########
@@ -81,12 +83,14 @@ void tearDown() throws IOException {
         client.close();
     }
 
-    @Override
+    @Disabled("JAMES-3492")
+	@Override
     protected void await() {
         elasticSearch.awaitForElasticSearch();
     }
 
-    @Override
+    @Disabled("JAMES-3492")

Review comment:
       This is not a test, no need to disable it




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


[GitHub] [james-project] Arsnael commented on pull request #288: James 3492

Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #288:
URL: https://github.com/apache/james-project/pull/288#issuecomment-780287317


   Oh and thank you very much for this contribution, the effort is hugely appreciated ! I think it's nearly mergeable, good work :)


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


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

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


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

Posted by GitBox <gi...@apache.org>.
mbaechler commented on a change in pull request #288:
URL: https://github.com/apache/james-project/pull/288#discussion_r571078614



##########
File path: backends-common/elasticsearch-v7/src/test/java/org/apache/james/backends/es/v7/ElasticSearchIndexerTest.java
##########
@@ -80,6 +81,7 @@ void tearDown() throws IOException {
     }
 
     @Test
+    @Disabled

Review comment:
       we usually put the message in @Disabled that refers to the ticket number linked to it. Here, you can use the JIRA 3492 so that we can search remaining tests to fix easily

##########
File path: mailbox/elasticsearch-v7/src/test/java/org/apache/james/mailbox/elasticsearch/v7/AbstractMessageSearchIndexTest.java
##########
@@ -0,0 +1,1599 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ m10 = otherInboxMessageManager.appendMessage(
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailbox.elasticsearch.v7;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.*;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.model.*;
+import org.apache.james.mailbox.model.SearchQuery.AddressType;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.mailbox.model.SearchQuery.Sort;
+import org.apache.james.mailbox.model.SearchQuery.Sort.Order;
+import org.apache.james.mailbox.model.SearchQuery.Sort.SortClause;
+import org.apache.james.mailbox.store.StoreMailboxManager;
+import org.apache.james.mailbox.store.StoreMessageManager;
+import org.apache.james.mailbox.store.search.MessageSearchIndex;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.dom.Multipart;
+import org.apache.james.mime4j.message.BodyPartBuilder;
+import org.apache.james.mime4j.message.MultipartBuilder;
+import org.apache.james.mime4j.message.SingleBodyBuilder;
+import org.apache.james.util.ClassLoaderUtils;
+import org.awaitility.Awaitility;
+import org.awaitility.Duration;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Flux;
+
+import javax.mail.Flags;
+import java.nio.charset.StandardCharsets;
+import java.text.SimpleDateFormat;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.List;
+import java.util.TimeZone;
+import java.util.concurrent.TimeUnit;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+@Disabled
+public abstract class AbstractMessageSearchIndexTest {

Review comment:
       usually we disable the tests in the implementation, not the abstract class.
   
   Just override the test you want to disable and put a @Disabled on the empty method and it solves your problem




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