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/12 09:48:16 UTC
[GitHub] [james-project] chibenwa opened a new pull request #293: JAMES-3500 Improve build stability
chibenwa opened a new pull request #293:
URL: https://github.com/apache/james-project/pull/293
----------------------------------------------------------------
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 #293: JAMES-3500 Improve build stability
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #293:
URL: https://github.com/apache/james-project/pull/293#discussion_r575196722
##########
File path: server/mailet/mock-smtp-server/src/main/java/org/apache/james/mock/smtp/server/testing/MockSmtpServerExtension.java
##########
@@ -43,7 +44,8 @@
DockerMockSmtp() {
mockSmtpServer = DockerContainer.fromName(Images.MOCK_SMTP_SERVER)
- .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()));
+ .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()))
+ .waitingFor(Wait.forLogMessage(".*Mock SMTP server started.*", 1));
Review comment:
HostPortWaitStrategy -> `Waits until a socket connection can be established on a port exposed or mapped by the container.`
I understand `any` not `all`.
A look at InternalCommandPortListeningCheck however tends to confirm what you say.
I will switch back to HostPortWaitStrategy and propose a PR upstream for fixing the ambiguous javadoc.
----------------------------------------------------------------
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 a change in pull request #293: JAMES-3500 Improve build stability
Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #293:
URL: https://github.com/apache/james-project/pull/293#discussion_r575312925
##########
File path: server/mailet/mock-smtp-server/src/main/java/org/apache/james/mock/smtp/server/testing/MockSmtpServerExtension.java
##########
@@ -43,7 +44,8 @@
DockerMockSmtp() {
mockSmtpServer = DockerContainer.fromName(Images.MOCK_SMTP_SERVER)
- .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()));
+ .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()))
+ .waitingFor(Wait.forLogMessage(".*Mock SMTP server started.*", 1));
Review comment:
that's weird: https://github.com/testcontainers/testcontainers-java/blob/17b4f6c136f6f2c7dc223bad407221f62a8f0088/core/src/main/java/org/testcontainers/containers/wait/strategy/HostPortWaitStrategy.java#L26 seems to use `getLivenessCheckPorts` (with an `s`) for the external checks and
https://github.com/testcontainers/testcontainers-java/blob/17b4f6c136f6f2c7dc223bad407221f62a8f0088/core/src/main/java/org/testcontainers/containers/wait/strategy/HostPortWaitStrategy.java#L35 uses `getExposedPorts` for the internal checks and before suggesting this I checked that this was already the case in 1.15.1
I don't see how you could use the container if both ports are not exposed since the tests call both the smtp and the http service ...
----------------------------------------------------------------
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 #293: JAMES-3500 Improve build stability
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #293:
URL: https://github.com/apache/james-project/pull/293#discussion_r575197203
##########
File path: server/mailet/mock-smtp-server/src/main/java/org/apache/james/mock/smtp/server/testing/MockSmtpServerExtension.java
##########
@@ -43,7 +44,8 @@
DockerMockSmtp() {
mockSmtpServer = DockerContainer.fromName(Images.MOCK_SMTP_SERVER)
- .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()));
+ .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()))
+ .waitingFor(Wait.forLogMessage(".*Mock SMTP server started.*", 1));
Review comment:
(yet fixing the container logs is still valuable ;-) )
----------------------------------------------------------------
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 #293: JAMES-3500 Improve build stability
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #293:
URL: https://github.com/apache/james-project/pull/293#discussion_r575327695
##########
File path: server/mailet/mock-smtp-server/src/main/java/org/apache/james/mock/smtp/server/testing/MockSmtpServerExtension.java
##########
@@ -43,7 +44,8 @@
DockerMockSmtp() {
mockSmtpServer = DockerContainer.fromName(Images.MOCK_SMTP_SERVER)
- .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()));
+ .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()))
+ .waitingFor(Wait.forLogMessage(".*Mock SMTP server started.*", 1));
Review comment:
My fault I did clone and looked at my long outdated fork instead of recent code. Sorry for the 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] jeantil commented on a change in pull request #293: JAMES-3500 Improve build stability
Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #293:
URL: https://github.com/apache/james-project/pull/293#discussion_r575183285
##########
File path: server/protocols/webadmin/webadmin-mailbox-deleted-message-vault/src/test/java/org/apache/james/webadmin/vault/routes/DeletedMessagesVaultRoutesTest.java
##########
@@ -216,7 +215,6 @@ void beforeEach() throws Exception {
.start();
RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(webAdminServer)
.setBasePath(DeletedMessagesVaultRoutes.ROOT_PATH)
- .log(LogDetail.METHOD)
Review comment:
thank you !
----------------------------------------------------------------
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 a change in pull request #293: JAMES-3500 Improve build stability
Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #293:
URL: https://github.com/apache/james-project/pull/293#discussion_r575182074
##########
File path: server/mailet/mock-smtp-server/src/main/java/org/apache/james/mock/smtp/server/testing/MockSmtpServerExtension.java
##########
@@ -43,7 +44,8 @@
DockerMockSmtp() {
mockSmtpServer = DockerContainer.fromName(Images.MOCK_SMTP_SERVER)
- .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()));
+ .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()))
+ .waitingFor(Wait.forLogMessage(".*Mock SMTP server started.*", 1));
Review comment:
hmm as far as I can tell, forListeningPorts waits for all exposed ports to answer I'm not sure why waiting for the log is better ...
----------------------------------------------------------------
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 #293: JAMES-3500 Improve build stability
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #293:
URL: https://github.com/apache/james-project/pull/293#issuecomment-778737994
Merged along #291 (just 2 commits were not included in 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] chibenwa commented on a change in pull request #293: JAMES-3500 Improve build stability
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #293:
URL: https://github.com/apache/james-project/pull/293#discussion_r575293815
##########
File path: server/mailet/mock-smtp-server/src/main/java/org/apache/james/mock/smtp/server/testing/MockSmtpServerExtension.java
##########
@@ -43,7 +44,8 @@
DockerMockSmtp() {
mockSmtpServer = DockerContainer.fromName(Images.MOCK_SMTP_SERVER)
- .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()));
+ .withLogConsumer(outputFrame -> LOGGER.debug("MockSMTP: " + outputFrame.getUtf8String()))
+ .waitingFor(Wait.forLogMessage(".*Mock SMTP server started.*", 1));
Review comment:
Correction, I got misleaded by uncompiled code, testcontainer uses a single port for live check (`GenericContainer::getLivenessCheckPort`) I gonna revert the fixup and switch back to the log check.
----------------------------------------------------------------
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 closed pull request #293: JAMES-3500 Improve build stability
Posted by GitBox <gi...@apache.org>.
chibenwa closed pull request #293:
URL: https://github.com/apache/james-project/pull/293
----------------------------------------------------------------
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