533314 - Tests: Use [e-mail] instead of [e-mail]

Status: RESOLVED FIXED

(MailNews Core :: Testing Infrastructure, defect)

Description

[Ben Bucksch (:BenB)]

test_bug474774.js and many many others:

Actual:
const kSender = "[e-mail]";
const kTo = "[e-mail]";

Expected:
const kTo = "[e-mail]";

Reason:
RFC 2606

Comment 1

[Saint Wesonga]

Attachment: Patch

Comment 2

[Dan Mosedale (:dmosedale, :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?

Comment 3

[Mark Banner (:standard8)]

(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.

Comment 4

[Ben Bucksch (:BenB)]

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

Comment 5

[Ben Bucksch (:BenB)]

(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

[David :Bienvenu]

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

[:aceman]

Why is this in composition component?

Saint Wesonga, are you going to finish this patch?

Comment 8

[:aceman]

Aryx, maybe this would be some relaxing for you :)

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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).

Comment 10

[Kent James (:rkent)]

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.

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: make used mail addresses invalid, new patch v1

Successful Thunderbird-Try run: https://tbpl.mozilla.org/?tree=Thunderbird-Try&noignore=1&rev=ae8eb8a94a36

Comment 12

[Mark Banner (:standard8)]

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.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: make used mail addresses invalid, v2 (bitrot fixed), r=Standard8

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: make used mail addresses invalid, v3 (bitrot fixed and removed changes which sneaked in), r=Standard8

Comment 15

[Ryan VanderMeulen [:RyanVM]]

This doesn't apply cleanly (yeah, I know). Please rebase (and don't forget to qref!).

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: 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.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/af56cbb73657