Closed Bug 1140032 Opened 5 years ago Closed 5 years ago

Re-enable FTP/cache xpcshell tests

Categories

(Core :: Networking: FTP, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: mayhemer, Assigned: emk)

Details

Attachments

(2 files, 1 obsolete file)

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 v2Splinter 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 patchSplinter 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: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.