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 2020/12/03 22:13:45 UTC
[GitHub] [james-project] jeantil opened a new pull request #270: JAMES-2543 Migrate MPT SMTP tests to junit jupiter
jeantil opened a new pull request #270:
URL: https://github.com/apache/james-project/pull/270
----------------------------------------------------------------
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] jeantil commented on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738667436
>Usually we remove the public annotation for the test methods because they are not required. Could you handle it?
fixed up :)
----------------------------------------------------------------
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] rouazana commented on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
rouazana commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738670812
> fixed up :)
Thanks that was quick :) What about SMTPStartTlsResponseTest, DNSRBLHandlerTest and more globally all the files modified in the second commit?
----------------------------------------------------------------
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 #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738631030
> Nice, thanks a ton!
^^ @jeantil historically it had been mostly @Arsnael contributing on this topic.
----------------------------------------------------------------
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] jeantil commented on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738675686
> I fear the trickiest place remains ^^ (server/mailet/integration-test for instance is going to be challenging!)
```
> grep -A1 -rh "@Rule" server/mailet/integration-testing | sort | uniq
--
public AmqpRule amqpRule = new AmqpRule(rabbitMqContainer, EXCHANGE_NAME, ROUTING_KEY);
public ExpectedException expectedException = ExpectedException.none();
public final RuleChain chain = RuleChain.outerRule(temporaryFolder).around(rabbitMqContainer).around(amqpRule);
public RuleChain chain = RuleChain.outerRule(rabbit).around(amqpRule).around(folder);
public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
public SMTPMessageSender smtpMessageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
public StaticInputChecker resultChecker = new StaticInputChecker();
public TemporaryFolder folder = new TemporaryFolder();
public TemporaryFolder temporaryFolder = new TemporaryFolder();
public TestIMAPClient messageReader = new TestIMAPClient();
public TestIMAPClient testIMAPClient = new TestIMAPClient();
```
* AmqpRule -> RabbitMqExtension ?
* ExpectedException -> unsure but I would be very surprised if junit 5 doesn't have an equivalent
* Rule chains are replaced by declaration order
* SMTPMessageSender -> SMTPMessageSenderExtension
* TemporaryFolder -> TemporaryFolderExtension
that leaves :
* StaticInputChecker
* TestIMAPClient
followed by a bit of mindless grunt work to manually replace all the @Rule by the corresponding extensions and then a
mass search/replace :
* import org.junit.After -> import org.junit.jupiter.api.AfterEach ;
* import org.junit.Before -> import org.junit.jupiter.api.BeforeEach;
* import org.junit.Test -> import org.junit.jupiter.api.Rule;
Followed by a mass cleanup imports on the module (thank you intellij)
this could very well be tagged as an easy contribution for external contributors especially if the missing extensions have been built beforehand by more experiences members :)
----------------------------------------------------------------
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] jeantil closed pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
jeantil closed pull request #270:
URL: https://github.com/apache/james-project/pull/270
----------------------------------------------------------------
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] jeantil commented on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738741898
there you go 2 fixups to their respective commits to remove public modifier on test methods
----------------------------------------------------------------
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] jeantil commented on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-739772312
>Can you fix them in another PR?
yes
----------------------------------------------------------------
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] jeantil commented on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738676986
@rouazana sorry I had initially focused on mpt and forgot that I extended the modifications to the protocol modules :) actualy I fixed up to the wrong commit because of this. there are more files involved in protocol smtp it would exceed pause time and will have to wait for the next pause .
----------------------------------------------------------------
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 edited a comment on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
Arsnael edited a comment on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738729290
> ^^ @jeantil historically it had been mostly @Arsnael contributing on this topic.
Yes but I never took the time to completely finish those. So big thanks to you @jeantil :)
----------------------------------------------------------------
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] rouazana commented on a change in pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
rouazana commented on a change in pull request #270:
URL: https://github.com/apache/james-project/pull/270#discussion_r536413848
##########
File path: mpt/impl/smtp/core/src/main/java/org/apache/james/mpt/smtp/SmtpStarttlsCommandTest.java
##########
@@ -33,47 +31,42 @@
public static final String USER_AT_DOMAIN = USER + "@" + DOMAIN;
public static final String PASSWORD = "secret";
- @Rule
- public final TemporaryFolder folder = new TemporaryFolder();
-
- protected abstract SmtpHostSystem createSmtpHostSystem();
-
- private SmtpHostSystem hostSystem;
+ protected SmtpHostSystem hostSystem;
private SimpleScriptedTestProtocol scriptedTest;
- @Before
- public void setUp() throws Exception {
- hostSystem = createSmtpHostSystem();
+ @BeforeEach
+ public void setUp(SmtpHostSystem hostSystem) throws Exception {
Review comment:
```suggestion
void setUp(SmtpHostSystem hostSystem) throws Exception {
```
##########
File path: server/testing/src/main/java/org/apache/james/utils/FakeSmtpExtension.java
##########
@@ -0,0 +1,168 @@
+/****************************************************************
+ * 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 *
+ * *
+ * 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.utils;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.config.EncoderConfig.encoderConfig;
+import static io.restassured.config.RestAssuredConfig.newConfig;
+
+import java.nio.charset.StandardCharsets;
+import java.time.Duration;
+import java.util.function.Consumer;
+
+import org.apache.james.util.docker.Images;
+import org.apache.james.util.docker.RateLimiters;
+import org.junit.jupiter.api.extension.AfterEachCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
+import org.junit.jupiter.api.extension.ParameterResolutionException;
+import org.junit.jupiter.api.extension.ParameterResolver;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.containers.wait.strategy.HostPortWaitStrategy;
+
+import com.github.dockerjava.api.model.ContainerNetwork;
+
+import io.restassured.builder.RequestSpecBuilder;
+import io.restassured.builder.ResponseSpecBuilder;
+import io.restassured.http.ContentType;
+import io.restassured.response.ValidatableResponse;
+import io.restassured.specification.RequestSpecification;
+import io.restassured.specification.ResponseSpecification;
+
+
+public class FakeSmtpExtension implements
+ BeforeEachCallback,
+ AfterEachCallback,
+ ParameterResolver {
+
+ private static final int SMTP_PORT = 25;
+
+ public static FakeSmtpExtension withSmtpPort(Integer smtpPort) {
+ GenericContainer<?> container = fakeSmtpContainer()
+ .withExposedPorts(smtpPort)
+ .withCommand("node", "cli", "--listen", "80", "--smtp", smtpPort.toString());
+
+ return new FakeSmtpExtension(container);
+ }
+
+ public static FakeSmtpExtension withDefaultPort() {
+ return withSmtpPort(SMTP_PORT);
+ }
+
+ private static GenericContainer<?> fakeSmtpContainer() {
Review comment:
```suggestion
private static GenericContainer<?> fakeSmtpContainer() {
```
----------------------------------------------------------------
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] jeantil commented on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738642038
last count in https://issues.apache.org/jira/browse/JAMES-2543 was 477, with this we get to
```
> grep -r -F 'import org.junit.Test'|wc -l
392
```
almost there !
----------------------------------------------------------------
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] jeantil edited a comment on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
jeantil edited a comment on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738667436
> Usually we remove the public annotation for the test methods because they are not required. Could you handle it?
fixed up :)
----------------------------------------------------------------
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 #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-739090146
```
[61b18c2de411a46bd812334b8830cd36477dcd3c] [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:testCompile (default-testCompile) on project protocols-lmtp: Compilation failure: Compilation failure:
[61b18c2de411a46bd812334b8830cd36477dcd3c] [ERROR] /james-project/protocols/lmtp/src/test/java/org/apache/james/protocols/lmtp/AbstractLMTPServerTest.java:[70,5] method does not override or implement a method from a supertype
[61b18c2de411a46bd812334b8830cd36477dcd3c] [ERROR] /james-project/protocols/lmtp/src/test/java/org/apache/james/protocols/lmtp/AbstractLMTPServerTest.java:[75,5] method does not override or implement a method from a supertype
[61b18c2de411a46bd812334b8830cd36477dcd3c] [ERROR] /james-project/protocols/lmtp/src/test/java/org/apache/james/protocols/lmtp/AbstractLMTPServerTest.java:[81,5] method does not override or implement a method from a supertype
[61b18c2de411a46bd812334b8830cd36477dcd3c] [ERROR] /james-project/protocols/lmtp/src/test/java/org/apache/james/protocols/lmtp/AbstractLMTPServerTest.java:[87,5] method does not override or implement a method from a supertype
[61b18c2de411a46bd812334b8830cd36477dcd3c] [ERROR] /james-project/protocols/lmtp/src/test/java/org/apache/james/protocols/lmtp/AbstractLMTPServerTest.java:[92,5] method does not override or implement a method from a supertype
[61b18c2de411a46bd812334b8830cd36477dcd3c] [ERROR] /james-project/protocols/lmtp/src/test/java/org/apache/james/protocols/lmtp/AbstractLMTPServerTest.java:[127,5] method does not override or implement a method from a supertype
```
The build fails as LMTP has a dependency on SMTP
----------------------------------------------------------------
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 #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738729290
> ^^ @jeantil historically it had been mostly @Arsnael contributing on this topic.
Yes but I never took the time to completely finish those. So big thanks to you @jeantil :)
----------------------------------------------------------------
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] jeantil commented on pull request #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-739738876
@chibenwa thanks, I had not seen the last 2 suggestions of @rouazana (extra space and leftover public on a setup method) did you fix them as part of the merge ? (otherwise i'll do a followup MR to cleanup my mess)
----------------------------------------------------------------
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 #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-739746917
> I had not seen the last 2 suggestions of @rouazana (extra space and leftover public on a setup method) did you fix them as part of the merge ? (otherwise i'll do a followup MR to cleanup my mess)
I've not been careful enough sorry.
Can you fix them in another PR?
----------------------------------------------------------------
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 #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-738643801
> almost there !
I fear the trickiest place remains ^^ (server/mailet/integration-test for instance is going to be challenging!)
----------------------------------------------------------------
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 #270: JAMES-2543 Migrate SMTP tests to junit 5
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #270:
URL: https://github.com/apache/james-project/pull/270#issuecomment-739694341
Hello Jean, I merged this nice 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