Last Comment Bug 686429 - test cases for MIME headers need refactoring
: test cases for MIME headers need refactoring
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla10
Assigned To: Julian Reschke
:
Mentors:
Depends on: 610054
Blocks: 609667
  Show dependency treegraph
 
Reported: 2011-09-13 01:14 PDT by Julian Reschke
Modified: 2011-10-13 07:21 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed patch (20.46 KB, patch)
2011-10-08 10:42 PDT, Julian Reschke
bzbarsky: review+
Details | Diff | Review
Proposed patch (20.46 KB, patch)
2011-10-12 00:49 PDT, Julian Reschke
bzbarsky: review+
Details | Diff | Review

Description Julian Reschke 2011-09-13 01:14:56 PDT
(1) They feed the header name into the call, but the function being tested expects only the feed value. That the tests seem to do what they should do seems to be pure luck.

(2) Currently the tests reside in multiple JS files due to historic reasons; consolidate them in a single file.
Comment 1 Julian Reschke 2011-10-08 08:18:42 PDT
(In reply to Julian Reschke from comment #0)
> (1) They feed the header name into the call, but the function being tested
> expects only the feed value. That the tests seem to do what they should do
> seems to be pure luck.
> ...

For the base class, this was already done for bug 610054.
Comment 2 Julian Reschke 2011-10-08 10:42:13 PDT
Created attachment 565745 [details] [diff] [review]
Proposed patch

Consolidates all Content-Disposition tests in test_MIME_params.js.

Also: all tests check both the filename and the disposition type.
Comment 3 Julian Reschke 2011-10-08 10:43:10 PDT
Comment on attachment 565745 [details] [diff] [review]
Proposed patch

(patch is on top of 610054)
Comment 4 Boris Zbarsky [:bz] 2011-10-11 20:44:31 PDT
Comment on attachment 565745 [details] [diff] [review]
Proposed patch

> +//  (note: 5987-mode node yet in use, see bug 601933)

"note yet in use", yes?

r=me
Comment 5 Julian Reschke 2011-10-12 00:49:39 PDT
Created attachment 566460 [details] [diff] [review]
Proposed patch

Proposed patch with typo in comment fixed ("node" ->" not")
Comment 6 :Ms2ger 2011-10-12 13:23:46 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/ceb6b4df17d5

FWIW, having your name and the reviewer in the patch is helpful.
Comment 7 Marco Bonardo [::mak] 2011-10-13 07:21:06 PDT
https://hg.mozilla.org/mozilla-central/rev/ceb6b4df17d5

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