Closed
Bug 887910
Opened 12 years ago
Closed 12 years ago
mozfile's test_load.py:test_http shouldn't use hardcoded port for http server
Categories
(Testing :: Mozbase, defect)
Testing
Mozbase
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: wlach, Assigned: ffledgling)
Details
Attachments
(1 file, 1 obsolete file)
1.11 KB,
patch
|
wlach
:
review+
|
Details | Diff | Splinter Review |
https://github.com/mozilla/mozbase/blob/master/mozfile/tests/test_load.py#L28
By always instantiating the server with port 8888, we create two problems:
1. If another test wants to grab port 8888 at the same time, it will fail.
2. If you have a server listening on port 8888 when the test runs, it will fail.
The preferred use of mozhttpd is to pass in port 0 (now the default if nothing else specified). Let's fix the test to do that, and use the new get_url method (http://mozbase.readthedocs.org/en/latest/mozhttpd.html#mozhttpd.MozHttpd.get_url) to get the file url below while we're at it.
Assignee | ||
Comment 1•12 years ago
|
||
I've removed the port parameter specified in the test entirely, since 0 is now the default and since we're using get_url we should not really be worried about the specified port, will that be okay or should I specify port=0 explicitly?
Attachment #771336 -
Flags: review?(wlachance)
Assignee | ||
Updated•12 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Reporter | ||
Comment 2•12 years ago
|
||
Comment on attachment 771336 [details] [diff] [review]
Patch to remove hard-coded port usage
Review of attachment 771336 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good with some minor requests for changes. r+ with those applied.
::: mozfile/tests/test_load.py
@@ +33,5 @@
> httpd.start(block=False)
> + # Get the server's url
> + server_url = httpd.get_url()
> + # Fetch the content from the url
> + content = load(server_url).read()
I would just collapse the code + comments into:
content = load(httpd.get_url()).read()
It's pretty obvious what's going on without the comment.
@@ +34,5 @@
> + # Get the server's url
> + server_url = httpd.get_url()
> + # Fetch the content from the url
> + content = load(server_url).read()
> + print content
Remove this line.
Attachment #771336 -
Flags: review?(wlachance) → review+
Reporter | ||
Comment 3•12 years ago
|
||
Oh, and no, there's no need to specify the port anymore.
Assignee | ||
Comment 4•12 years ago
|
||
Updated with fixes
Attachment #771336 -
Attachment is obsolete: true
Attachment #771379 -
Flags: review?(wlachance)
Reporter | ||
Updated•12 years ago
|
Attachment #771379 -
Flags: review?(wlachance) → review+
Reporter | ||
Comment 5•12 years ago
|
||
Reporter | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•