Tests: Use foo@foo.invalid instead of foo@invalid.com

RESOLVED FIXED in Thunderbird 21.0

Status

MailNews Core
Testing Infrastructure
--
minor
RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: BenB, Assigned: aryx)

Tracking

Trunk
Thunderbird 21.0
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

8 years ago
test_bug474774.js and many many others:

Actual:
const kSender = "from@invalid.com";
const kTo = "to@invalid.com";

Expected:
const kTo = "to@foo.invalid";

Reason:
RFC 2606
Assignee: bugzilla → nobody
Keywords: helpwanted
Whiteboard: [good first bug]

Comment 1

8 years ago
Created attachment 417417 [details] [diff] [review]
Patch
Assignee: nobody → wesongathedeveloper
Status: NEW → ASSIGNED
Attachment #417417 - Flags: review?(dmose)
While the changes in this patch _look_ right from reading them, they don't actually apply because the original .eml files in mercurial have DOS line endings ("/r/n"), and this patch itself does not.  It's not exactly clear to me how the problem got introduced here.  It could be the fault of the OS you're running on, Mercurial, or maybe even the editor (though that less seems less likely).  Saint Wesonga, would you try to figure out how to generate a patch that doesn't have that problem?

The .eml files in Mercurial contain the text "this email is in dos format because that is what the interface requires", which appears to have originated when Standard8 originally checked the first version of CVS.  Standard8, do you know you exactly what "the interface" is referring to?

Updated

8 years ago
Attachment #417417 - Flags: review?(dmose) → review-
(In reply to comment #2)
> The .eml files in Mercurial contain the text "this email is in dos format
> because that is what the interface requires", which appears to have originated
> when Standard8 originally checked the first version of CVS.  Standard8, do you
> know you exactly what "the interface" is referring to?

"The interfaces" is more referring to the format we typically store and process our emails in - which is the dos format, even on non-dos type platforms.
(Reporter)

Comment 4

8 years ago
Say "data format" and "wire line-endings" (CR LF, see RFC 2821), and it's logical why that's expected.
(Reporter)

Comment 5

8 years ago
(Another way would of course be to let the testsuite adapt the lineendings from platform to wire, but that'd be offtopic here.)

Comment 6

8 years ago
I could be wrong, but I believe the issue here is that mail servers (pop3, imap) are supposed to return CRLF terminated lines, and since our fake servers don't guarantee that, it's just easier if we give them CRLF terminated messages to being with.

Comment 7

5 years ago
Why is this in composition component?

Saint Wesonga, are you going to finish this patch?
Flags: needinfo?(wesongathedeveloper)

Updated

4 years ago
Status: ASSIGNED → NEW

Updated

4 years ago
Assignee: wesongathedeveloper → nobody

Updated

4 years ago
Assignee: nobody → acelists
Severity: trivial → minor
Component: Composition → Testing Infrastructure
Flags: needinfo?(wesongathedeveloper)
Version: 1.9.1 Branch → Trunk

Comment 8

4 years ago
Aryx, maybe this would be some relaxing for you :)
Assignee: acelists → archaeopteryx
To complete the patch, I need to know how to edit mailnews/extensions/bayesian-spam-filter/test/unit/msgCorpus.dat (a renamed training.dat with junk filter training data).
Flags: needinfo?(kent)

Comment 10

4 years ago
Changing that is not trivial. It was generated from a real test message that I sent to myself.

Why do you need to change it? There is no invalid.com there. yes hostmonster.com and caspia.com, but these are only used as text strings in this test.
Flags: needinfo?(kent)
Created attachment 697401 [details] [diff] [review]
make used mail addresses invalid, new patch v1

Successful Thunderbird-Try run: https://tbpl.mozilla.org/?tree=Thunderbird-Try&noignore=1&rev=ae8eb8a94a36
Attachment #417417 - Attachment is obsolete: true
Attachment #697401 - Flags: review?(mbanner)
Status: NEW → ASSIGNED
Comment on attachment 697401 [details] [diff] [review]
make used mail addresses invalid, new patch v1

Sorry for the delay, looks good. There's a little bit of bitrot that'll need to be fixed before checkin.
Attachment #697401 - Flags: review?(mbanner) → review+
Created attachment 711357 [details] [diff] [review]
make used mail addresses invalid, v2 (bitrot fixed), r=Standard8
Attachment #697401 - Attachment is obsolete: true
Keywords: helpwanted → checkin-needed
Whiteboard: [good first bug] → [check in to comm-central]
Created attachment 711368 [details] [diff] [review]
make used mail addresses invalid, v3 (bitrot fixed and removed changes which sneaked in), r=Standard8
Attachment #711357 - Attachment is obsolete: true
This doesn't apply cleanly (yeah, I know). Please rebase (and don't forget to qref!).
Keywords: checkin-needed
Created attachment 711431 [details] [diff] [review]
make used mail addresses invalid, v4, r=Standard8

Sorry for the confusion.

TL;DR: Fixed in this patch.

Extended version: I hadn't qref-ed when uploading the patch for review, leaving out two fixes without two tests fail. After uploading, I discovered a file size discrepancies between the new and an old export of the patch and I didn't correct that properly in the patch submitted earlier.
Attachment #711368 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/af56cbb73657
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [check in to comm-central]
Target Milestone: --- → Thunderbird 21.0
You need to log in before you can comment on or make changes to this bug.