Closed Bug 1672296 Opened 11 months ago Closed 11 months ago

C-C TB mochitest produces "CardDAV username not set. This probably means you set up CardDAV before Thunderbird 82. Remove the directory and set it up again, or set pref ldap_2.servers.sync.carddav.username with your username."

Categories

(Thunderbird :: Address Book, defect)

x86_64
Linux
defect

Tracking

(thunderbird_esr78 fixed, thunderbird83 affected)

RESOLVED FIXED
84 Branch
Tracking Status
thunderbird_esr78 --- fixed
thunderbird83 --- affected

People

(Reporter: ishikawa, Assigned: mkmelin)

Details

Attachments

(1 file)

I think C-C TB's mochitest does not set up test for CardDAV correctly.

I see the following message in my local mochitest log produced by
running FULL debug version of C-C TB.

console.error: "CardDAV username not set. This probably means you set up CardDAV before Thunderbird 82. Remove the directory and set it up again, or set pref ldap_2.servers.sync.carddav.username with your username."

This is relatively new, I don't see it in the local log dated Sept 29.

The test that produces the message is
comm/mail/components/addrbook/test/browser/browser_cardDAV_sync.js

The same error is visible in other people's try-comm-centraal server
run as well.
For example, see the linux 64 debug version test runs.
https://firefoxci.taskcluster-artifacts.net/FEDstsZzS3i8V1gzvfyIlA/0/public/logs/live_backing.log
https://firefoxci.taskcluster-artifacts.net/fNL0NMVYR4qiNyIpO7ajpg/0/public/logs/live_backing.log

Since I am running the test for the first time after updating the
M-C/C-C tree, I would assume that the initialization code in the test
is faulty.

It isn't faulty. The server used by the test doesn't require a username and password, so none is set up.

(In reply to Geoff Lankow (:darktrojan) from comment #1)

It isn't faulty. The server used by the test doesn't require a username and password, so none is set up.

Aha, I see.
It would be great if ”comm/mail/components/addrbook/test/browser/browser_cardDAV_sync.js" prints something like

We expect the following error message to the console, but it is due to the test server not requiring the user account. So you can ignore them:
 (quote the error message verbatim)

I think, in the long run, we should have some kind of registering (test file, error message) pair n a whitelist and log analyzer with filtering based on such whitelist.

At this point, doesn't seem like we should warn about this (and certainly not error output about it).

Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #9183111 - Flags: review?(geoff)

I am curious.
Will there be some incompatibility between pre-82 and post-82 TB regarding this behavior?
(That was implied by the error message.

Another thing is, how do we make sure that such warning/error message be reinstated once 82 comes along?
In that sense, it may be better simply commenting it out instead of removing the lines, no?
There is a chance of someone noticing it if we forget to reinstate it.

Comment on attachment 9183111 [details] [diff] [review]
bug1672296_carddav_username_warn.patch

Okay. It's probably been long enough now.

Attachment #9183111 - Flags: review?(geoff) → review+

(In reply to ISHIKAWA, Chiaki from comment #4)
It's only ever been behind a scary pref, so caveat emptor.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/ea906a36b774
don't console.error about missing username for CardDAV - anonymous access is a valid use case. r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED

Probably don't want to uplift.

Target Milestone: --- → 84 Branch

Comment on attachment 9183111 [details] [diff] [review]
bug1672296_carddav_username_warn.patch

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: just removes a logged warning
Testing completed (on c-c, etc.): on beta since 84
Risk to taking this patch (and alternatives if risky): no

See https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=e39c84abd9c0421bc2fa5c9ad6da315fd2195cab for landing order.

Attachment #9183111 - Flags: approval-comm-esr78?

Comment on attachment 9183111 [details] [diff] [review]
bug1672296_carddav_username_warn.patch

[Triage Comment]
Approved for esr78 (cardDav set)

Attachment #9183111 - Flags: approval-comm-esr78? → approval-comm-esr78+
You need to log in before you can comment on or make changes to this bug.