Last Comment Bug 533314 - Tests: Use foo@foo.invalid instead of foo@invalid.com
: Tests: Use foo@foo.invalid instead of foo@invalid.com
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Testing Infrastructure (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 21.0
Assigned To: Sebastian H. [:aryx][:archaeopteryx]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-07 11:18 PST by Ben Bucksch (:BenB)
Modified: 2013-02-07 12:41 PST (History)
9 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (57.51 KB, patch)
2009-12-13 18:04 PST, Saint Wesonga
dmose: review-
Details | Diff | Splinter Review
make used mail addresses invalid, new patch v1 (138.55 KB, patch)
2013-01-03 04:24 PST, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
make used mail addresses invalid, v2 (bitrot fixed), r=Standard8 (139.76 KB, patch)
2013-02-07 08:17 PST, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
make used mail addresses invalid, v3 (bitrot fixed and removed changes which sneaked in), r=Standard8 (138.45 KB, patch)
2013-02-07 08:32 PST, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
make used mail addresses invalid, v4, r=Standard8 (139.76 KB, patch)
2013-02-07 11:22 PST, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review

Description Ben Bucksch (:BenB) 2009-12-07 11:18:55 PST
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
Comment 1 Saint Wesonga 2009-12-13 18:04:37 PST
Created attachment 417417 [details] [diff] [review]
Patch
Comment 2 Dan Mosedale (:dmose) 2009-12-18 19:54:09 PST
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) (afk until 26th July) 2009-12-20 13:59:29 PST
(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) 2009-12-20 17:43:26 PST
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) 2009-12-20 17:44:39 PST
(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 2009-12-20 17:47:00 PST
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 2012-11-25 04:36:33 PST
Why is this in composition component?

Saint Wesonga, are you going to finish this patch?
Comment 8 :aceman 2012-12-22 11:17:49 PST
Aryx, maybe this would be some relaxing for you :)
Comment 9 Sebastian H. [:aryx][:archaeopteryx] 2012-12-28 04:58:40 PST
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) 2012-12-31 11:30:25 PST
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 H. [:aryx][:archaeopteryx] 2013-01-03 04:24:18 PST
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
Comment 12 Mark Banner (:standard8) (afk until 26th July) 2013-02-07 00:51:47 PST
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 H. [:aryx][:archaeopteryx] 2013-02-07 08:17:06 PST
Created attachment 711357 [details] [diff] [review]
make used mail addresses invalid, v2 (bitrot fixed), r=Standard8
Comment 14 Sebastian H. [:aryx][:archaeopteryx] 2013-02-07 08:32:00 PST
Created attachment 711368 [details] [diff] [review]
make used mail addresses invalid, v3 (bitrot fixed and removed changes which sneaked in), r=Standard8
Comment 15 Ryan VanderMeulen [:RyanVM] 2013-02-07 09:43:43 PST
This doesn't apply cleanly (yeah, I know). Please rebase (and don't forget to qref!).
Comment 16 Sebastian H. [:aryx][:archaeopteryx] 2013-02-07 11:22:46 PST
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.
Comment 17 Ryan VanderMeulen [:RyanVM] 2013-02-07 12:41:04 PST
https://hg.mozilla.org/comm-central/rev/af56cbb73657

Note You need to log in before you can comment on or make changes to this bug.