Closed Bug 1140032 Opened 11 years ago Closed 11 years ago

Re-enable FTP/cache xpcshell tests

Categories

(Core Graveyard :: Networking: FTP, defect)

Product:
Core Graveyard
Component:
Networking: FTP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox39 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox39 --- fixed

People

(Reporter: mayhemer, Assigned: emk)

Details

Attachments

(2 files, 1 obsolete file)

v2
17.98 KB, patch
jduell.mcbugs
: review+
Details | Diff | Splinter Review
patch
13.35 KB, patch
mayhemer
: review+
Details | Diff | Splinter Review
It's an FTP test that is injecting cache entries. It's disabled now anyway. FTP caching support has been removed recently.
Attached patch v1 (obsolete) — — Splinter Review
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attachment #8573402 - Flags: review?(dd.mozilla)
There are more!
Summary: Remove test_bug365133.js → Remove FTP/cache xpcshell tests
Attachment #8573402 - Attachment is obsolete: true
Attachment #8573402 - Flags: review?(dd.mozilla)
Attached patch v2 — — Splinter Review
I'll ask Jason here, since this is more a political decision. These test are checking regressions for actual bugs, but since we don't have an ftp server we let FTP channels read cache entries instead. These test were tho long time disabled and we don't invest to FTP code any resources anyway. So I think we are OK to remove these tests.
Attachment #8573427 - Flags: review?(jduell.mcbugs)
Comment on attachment 8573427 [details] [diff] [review] v2 Review of attachment 8573427 [details] [diff] [review]: ----------------------------------------------------------------- We're not keeping FTP using the cache just so we can synthesize test channels.
Attachment #8573427 - Flags: review?(jduell.mcbugs) → review+
Will you accept a patch if I make these tests work without using the cache?
Flags: needinfo?(honzab.moz)
(In reply to Masatoshi Kimura [:emk] from comment #5) > Will you accept a patch if I make these tests work without using the cache? Oh, definitely! Thanks. Please feel free to take the bug.
Flags: needinfo?(honzab.moz)
OK, taking.
Assignee: honzab.moz → VYV03354
Summary: Remove FTP/cache xpcshell tests → Re-enable FTP/cache xpcshell tests
Attached patch patch — — Splinter Review
We have never used a real FTP server. These are unit tests, after all. https://treeherder.mozilla.org/#/jobs?repo=try&revision=50731b7d4c7f
Attachment #8574867 - Flags: review?(honzab.moz)
:emk, some overview description of what your changes do would be nice...
Comment on attachment 8574867 [details] [diff] [review] patch Review of attachment 8574867 [details] [diff] [review]: ----------------------------------------------------------------- Very nice idea, simulate a generic channel and pass it to the converter directly. Nice! Thanks. r=honzab
Attachment #8574867 - Flags: review?(honzab.moz) → review+
Yes, I created a mock to simulate a channel. https://hg.mozilla.org/integration/mozilla-inbound/rev/530055ae5b49
https://hg.mozilla.org/mozilla-central/rev/530055ae5b49
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
status-firefox39: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: