You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "woj-tek (via GitHub)" <gi...@apache.org> on 2023/03/10 22:41:42 UTC
[GitHub] [james-project] woj-tek opened a new pull request, #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
woj-tek opened a new pull request, #1476:
URL: https://github.com/apache/james-project/pull/1476
I opted to create default mailboxes when `INBOX` is created without explicitly checking if they exist to lower the impact on the repositories as the check is made on each logging in.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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 #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1468365078
org.apache.james.jmap.rabbitmq.RabbitMQAwsS3GetMailboxesMethodTest.getMailboxesShouldReturnAllMailboxesWhenIdsIsNull
```
JSON path [0][1].list doesn't match.
Expected: a collection with size <8>
```
We need to add the Archive in the assertions of this test?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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 #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1475140297
Second approuval. @Arsnael maybe?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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 #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1473094157
> 1. `org.apache.james.mpt.imapmailbox.suite.Condstore#condstoreShouldBeEnableWhenGivenAndTrue` - this one seems to be unrelated to default mailboxes
Unfortunately I think it is... I ran that test on master branch -> success every time
I ran that test on your branch -> fails every time...
I tried to take a quick look at why it fails but I'm at a bit of a loss honestly. But it is related to the PR, I'm sorry.
> 2. tests in `org.apache.james.jmap.rfc8621`, for example `org.apache.james.jmap.rfc8621.contract.BackReferenceContract.resolvingAnUnexistingMethodCallIdShouldFail` - the difference in the length is obvious but the order of the list seems random at best?
Between `assertThatJson` and `isEqualTo` you can add the method `.withOptions(new Options(IGNORING_ARRAY_ORDER))`. It will ignore the order of the list (we didn't clean up everything related to that thanks for spotting 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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1474861243
It seems all tests are fixed now :-)
What's weird is that previous build failed suddenly with a bunch of mailet tests failing (https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1476/8/)...
On that note - is there a plan to somehow tackle the issue with the randomly failing test? and also printing lots of exceptions in the output? I saw there is `<excludedGroups>unstable</excludedGroups>` configuration already but I'm not sure that adding more test to that category "solves" the problem.
And the second thing - why some testing code is in the `main` directory` instead of `test`?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1474201916
> > 1. `org.apache.james.mpt.imapmailbox.suite.Condstore#condstoreShouldBeEnableWhenGivenAndTrue` - this one seems to be unrelated to default mailboxes
>
> Unfortunately I think it is... I ran that test on master branch -> success every time I ran that test on your branch -> fails every time...
I dug a bit more into it, and it seems it was caused by https://issues.apache.org/jira/browse/JAMES-2055 & https://github.com/linagora/james-project/pull/841/files and my change in testing when I used default global configuration but `Condstore` test was changing configuration as well (to check if disabling it works correctly). The issue was that if it got disabled once (as I did globally), it wasn't possible to re-enable it (which the `Condstore` was doing) hence the failing test. I'm not sure if reconfiguration is possible / expected, but I modified `CapabilityProcessor` so it's possible to disable/enable it on subsequent `configure()` calls and added unit test to verify it's working correctly.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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 #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1475140112
:green_apple: so I am merging this.
Thanks for the contribution.
Apache CI somethimes fails launching docker containers which causes multiple test to break. It was not stable lately but it seems now to be back on line.
> On that note - is there a plan to somehow tackle the issue with the randomly failing test? and also printing lots of exceptions in the output?
Yes. Contributors occasinally address some of them.
> I saw there is <excludedGroups>unstable</excludedGroups> configuration already but I'm not sure that adding more test to that category "solves" the problem.
Non critical unstable tests are excluded yes. Though this group shall be small ideally. :+1: for not adding more tests in there.
> And the second thing - why some testing code is in the main directoryinstead oftest`?
You have a specific example in mind?
We should not hesitate to move this discussion on the mailing list IMO.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on a diff in pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on code in PR #1476:
URL: https://github.com/apache/james-project/pull/1476#discussion_r1134558477
##########
mailbox/api/src/main/java/org/apache/james/mailbox/DefaultMailboxes.java:
##########
@@ -38,7 +38,7 @@ public interface DefaultMailboxes {
String TEMPLATES = "Templates";
String RESTORED_MESSAGES = "Restored-Messages";
- List<String> DEFAULT_MAILBOXES = ImmutableList.of(INBOX, OUTBOX, SENT, TRASH, DRAFTS, SPAM);
+ List<String> DEFAULT_MAILBOXES = ImmutableList.of(INBOX, OUTBOX, SENT, TRASH, DRAFTS, ARCHIVE, SPAM);
Review Comment:
Unfortunately I only run test of the modules I modified, mostly due to all suite with all integration test taking hours (probably https://issues.apache.org/jira/browse/JAMES-3879 would help with "test-ability" here :-) ).
What's the desired way forward - keeping ARCHIVE as default (I would say it's quite elementary folder) and identifying all those test or removing it from the list of defaults?
As for testing goes, I checked the build (https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1476/1/testReport/) and there are a lot of failures, but from the cursory look most of them are due to configuration not being set during tests causing nasty NPE so most likely a missing class/injection in the 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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1472805355
Looking at the history of the builds (https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1476/buildTimeTrend) the failure of the tests seems to be somewhat… random?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1467161360
There is a problem with lots of tests failing (which I mentioned in the in-line comment) - I checked the sources and if I get them right, it would seem that in order to fix those tests virtually all tests in `org.apache.james.mpt.imapmailbox` would have to be updated as they all rely on logging in. Alternatively I could set `imapConfiguration` in `ImapConfiguration` to `null` and do the null check in `org.apache.james.imap.processor.AbstractAuthProcessor#provisionInbox` but this would be just an ugly fix (workaround).
Btw. is `apache-james-mpt-imapmailbox-core` package used anywhere or is it only for tests? It looks like testing code but it's in `main` instead of `test` directory.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1473982971
> Unfortunately I think it is... I ran that test on master branch -> success every time I ran that test on your branch -> fails every time...
>
> I tried to take a quick look at why it fails but I'm at a bit of a loss honestly. But it is related to the PR, I'm sorry.
It's caused by this change https://github.com/apache/james-project/pull/1476/files#diff-1d7aee20797c7e30c6d737b31a644a146bb6e68bcd6ddbd599dd518dcf3da335R83 and then I made same changes in the problematic test: https://github.com/apache/james-project/pull/1476/files#diff-af9e56bd418cc71a19d530bb8b3d40b8d78f478dcbfaf6ec9355643fcfb466e5R65.
Most likely doing setting IMAP configuration in `JamesImapHostSystem` affects other IMAP processors hence difference in actual and expected list of capabilities. Will have to look more thoroughly into it.
> Between `assertThatJson` and `isEqualTo` you can add the method `.withOptions(new Options(IGNORING_ARRAY_ORDER))`. It will ignore the order of the list (we didn't clean up everything related to that thanks for spotting it)
Thanks! That did the trick :-)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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 merged pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael merged PR #1476:
URL: https://github.com/apache/james-project/pull/1476
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on a diff in pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on code in PR #1476:
URL: https://github.com/apache/james-project/pull/1476#discussion_r1134553367
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java:
##########
@@ -137,17 +147,29 @@ protected void provisionInbox(ImapSession session, MailboxManager mailboxManager
if (Mono.from(mailboxManager.mailboxExists(inboxPath, mailboxSession)).block()) {
LOGGER.debug("INBOX exists. No need to create it.");
} else {
- try {
- mailboxManager.createMailbox(inboxPath, MailboxManager.CreateOption.CREATE_SUBSCRIPTION, mailboxSession)
- .ifPresentOrElse(
- id -> LOGGER.info("Provisioning INBOX. {} created.", id),
- () -> LOGGER.warn("Provisioning INBOX successful. But no MailboxId have been returned."));
- } catch (MailboxExistsException e) {
- LOGGER.warn("Mailbox INBOX created by concurrent call. Safe to ignore this exception.");
+ provisionMailbox(DefaultMailboxes.INBOX, session, mailboxManager, mailboxSession);
+ if (imapConfiguration.isProvisionDefaultMailboxes()) {
+ for (String mailbox : DefaultMailboxes.DEFAULT_MAILBOXES) {
+ provisionMailbox(mailbox, session, mailboxManager, mailboxSession);
+ }
}
}
}
+ private void provisionMailbox(String mailbox, ImapSession session, MailboxManager mailboxManager,
+ MailboxSession mailboxSession) throws MailboxException {
+ var mailboxPath = PathConverter.forSession(session).buildFullPath(mailbox);
+ try {
+ mailboxManager.createMailbox(mailboxPath, MailboxManager.CreateOption.CREATE_SUBSCRIPTION, mailboxSession)
+ .ifPresentOrElse(id -> LOGGER.info("Provisioning mailbox {}. {} created.", mailbox, id),
+ () -> LOGGER.warn(
+ "Provisioning mailbox {} successful. But no MailboxId have been returned.",
+ mailbox));
+ } catch (MailboxExistsException e) {
+ LOGGER.warn("Mailbox {} created by concurrent call. Safe to ignore this exception.", mailbox);
+ }
Review Comment:
I pushed the change with the checking. Opted to repeat the check to keep it simple as it would most likely be run only once per account.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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 diff in pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1476:
URL: https://github.com/apache/james-project/pull/1476#discussion_r1133090905
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java:
##########
@@ -137,17 +147,29 @@ protected void provisionInbox(ImapSession session, MailboxManager mailboxManager
if (Mono.from(mailboxManager.mailboxExists(inboxPath, mailboxSession)).block()) {
LOGGER.debug("INBOX exists. No need to create it.");
} else {
- try {
- mailboxManager.createMailbox(inboxPath, MailboxManager.CreateOption.CREATE_SUBSCRIPTION, mailboxSession)
- .ifPresentOrElse(
- id -> LOGGER.info("Provisioning INBOX. {} created.", id),
- () -> LOGGER.warn("Provisioning INBOX successful. But no MailboxId have been returned."));
- } catch (MailboxExistsException e) {
- LOGGER.warn("Mailbox INBOX created by concurrent call. Safe to ignore this exception.");
+ provisionMailbox(DefaultMailboxes.INBOX, session, mailboxManager, mailboxSession);
+ if (imapConfiguration.isProvisionDefaultMailboxes()) {
+ for (String mailbox : DefaultMailboxes.DEFAULT_MAILBOXES) {
+ provisionMailbox(mailbox, session, mailboxManager, mailboxSession);
+ }
}
}
}
+ private void provisionMailbox(String mailbox, ImapSession session, MailboxManager mailboxManager,
+ MailboxSession mailboxSession) throws MailboxException {
+ var mailboxPath = PathConverter.forSession(session).buildFullPath(mailbox);
+ try {
+ mailboxManager.createMailbox(mailboxPath, MailboxManager.CreateOption.CREATE_SUBSCRIPTION, mailboxSession)
+ .ifPresentOrElse(id -> LOGGER.info("Provisioning mailbox {}. {} created.", mailbox, id),
+ () -> LOGGER.warn(
+ "Provisioning mailbox {} successful. But no MailboxId have been returned.",
+ mailbox));
+ } catch (MailboxExistsException e) {
+ LOGGER.warn("Mailbox {} created by concurrent call. Safe to ignore this exception.", mailbox);
+ }
Review Comment:
I bet that if the mailbox already exist prior to calling this method:
- That a `MailboxExistException` will be thrown
-and That we will have a misleading warning log.
My advice: call `mailboxExists` prior to creation to avoid both the exception and the log...
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java:
##########
@@ -137,17 +147,29 @@ protected void provisionInbox(ImapSession session, MailboxManager mailboxManager
if (Mono.from(mailboxManager.mailboxExists(inboxPath, mailboxSession)).block()) {
LOGGER.debug("INBOX exists. No need to create it.");
} else {
- try {
- mailboxManager.createMailbox(inboxPath, MailboxManager.CreateOption.CREATE_SUBSCRIPTION, mailboxSession)
- .ifPresentOrElse(
- id -> LOGGER.info("Provisioning INBOX. {} created.", id),
- () -> LOGGER.warn("Provisioning INBOX successful. But no MailboxId have been returned."));
- } catch (MailboxExistsException e) {
- LOGGER.warn("Mailbox INBOX created by concurrent call. Safe to ignore this exception.");
+ provisionMailbox(DefaultMailboxes.INBOX, session, mailboxManager, mailboxSession);
+ if (imapConfiguration.isProvisionDefaultMailboxes()) {
+ for (String mailbox : DefaultMailboxes.DEFAULT_MAILBOXES) {
+ provisionMailbox(mailbox, session, mailboxManager, mailboxSession);
+ }
}
}
}
+ private void provisionMailbox(String mailbox, ImapSession session, MailboxManager mailboxManager,
+ MailboxSession mailboxSession) throws MailboxException {
+ var mailboxPath = PathConverter.forSession(session).buildFullPath(mailbox);
+ try {
+ mailboxManager.createMailbox(mailboxPath, MailboxManager.CreateOption.CREATE_SUBSCRIPTION, mailboxSession)
+ .ifPresentOrElse(id -> LOGGER.info("Provisioning mailbox {}. {} created.", mailbox, id),
+ () -> LOGGER.warn(
+ "Provisioning mailbox {} successful. But no MailboxId have been returned.",
+ mailbox));
+ } catch (MailboxExistsException e) {
+ LOGGER.warn("Mailbox {} created by concurrent call. Safe to ignore this exception.", mailbox);
+ }
Review Comment:
I bet that if the mailbox already exist prior to calling this method:
- That a `MailboxExistException` will be thrown
- and That we will have a misleading warning log.
My advice: call `mailboxExists` prior to creation to avoid both the exception and the log...
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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 diff in pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1476:
URL: https://github.com/apache/james-project/pull/1476#discussion_r1133090483
##########
mailbox/api/src/main/java/org/apache/james/mailbox/DefaultMailboxes.java:
##########
@@ -38,7 +38,7 @@ public interface DefaultMailboxes {
String TEMPLATES = "Templates";
String RESTORED_MESSAGES = "Restored-Messages";
- List<String> DEFAULT_MAILBOXES = ImmutableList.of(INBOX, OUTBOX, SENT, TRASH, DRAFTS, SPAM);
+ List<String> DEFAULT_MAILBOXES = ImmutableList.of(INBOX, OUTBOX, SENT, TRASH, DRAFTS, ARCHIVE, SPAM);
Review Comment:
Pretty sure this will cause some tests to fail (JMAP mailbox provisionning).
Those tests would need to be fixed, too.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on a diff in pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on code in PR #1476:
URL: https://github.com/apache/james-project/pull/1476#discussion_r1134558477
##########
mailbox/api/src/main/java/org/apache/james/mailbox/DefaultMailboxes.java:
##########
@@ -38,7 +38,7 @@ public interface DefaultMailboxes {
String TEMPLATES = "Templates";
String RESTORED_MESSAGES = "Restored-Messages";
- List<String> DEFAULT_MAILBOXES = ImmutableList.of(INBOX, OUTBOX, SENT, TRASH, DRAFTS, SPAM);
+ List<String> DEFAULT_MAILBOXES = ImmutableList.of(INBOX, OUTBOX, SENT, TRASH, DRAFTS, ARCHIVE, SPAM);
Review Comment:
Unfortunately I only run test of the modules I modified, mostly due to all suite with all integration test taking hours (probably https://issues.apache.org/jira/browse/JAMES-3879 would help with "test-ability" here :-) ).
What's the desired way forward - keeping ARCHIVE as default (I would say it's quite elementary folder) and identifying all those test or removing it from the list of defaults?
As for testing goes, I checked the build (https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1476/1/testReport/) and there are a lot of failures, but from the cursory look most of them are due to configuration not being set during tests causing nasty NPE.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1468400277
Most likely, though I first wanted to address main issue that caused ~1400 tests fail, with that done (https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1476/3/) I'll go over remaining tests and see what the 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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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] woj-tek commented on pull request #1476: JAMES-3895 Automatically provision default (IMAP) mailboxes
Posted by "woj-tek (via GitHub)" <gi...@apache.org>.
woj-tek commented on PR #1476:
URL: https://github.com/apache/james-project/pull/1476#issuecomment-1472800579
I fixed some tests (relying on the actual value of `DefaultMailboxes.DEFAULT_MAILBOXES`), but there are some that still fails:
1) `org.apache.james.mpt.imapmailbox.suite.Condstore#condstoreShouldBeEnableWhenGivenAndTrue` - this one seems to be unrelated to default mailboxes
2) tests in `org.apache.james.jmap.rfc8621`, for example `org.apache.james.jmap.rfc8621.contract.BackReferenceContract.resolvingAnUnexistingMethodCallIdShouldFail` - the difference in the length is obvious but the order of the list seems random at best?
```
net.javacrumbs.jsonunit.core.internal.Opentest4jExceptionFactory$JsonAssertError: JSON documents are different:
Array "methodResponses[0][1].list" has different length, expected: <6> but was: <7>.
Array "methodResponses[0][1].list" has different content. Extra values: [{"id":"7"}], expected: <[{"id":"1"},{"id":"5"},{"id":"2"},{"id":"3"},{"id":"4"},{"id":"6"}]> but was: <[{"id":"1"},{"id":"6"},{"id":"5"},{"id":"2"},{"id":"3"},{"id":"4"},{"id":"7"}]>
Different value found in node "methodResponses[0][1].list[1].id", expected: <"5"> but was: <"6">.
Different value found in node "methodResponses[0][1].list[2].id", expected: <"2"> but was: <"5">.
Different value found in node "methodResponses[0][1].list[3].id", expected: <"3"> but was: <"2">.
Different value found in node "methodResponses[0][1].list[4].id", expected: <"4"> but was: <"3">.
Different value found in node "methodResponses[0][1].list[5].id", expected: <"6"> but was: <"4">.
```
Btw. One question, why some tests are in `main` and not in `tests` directories?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
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