Closed
Bug 887910
Opened 11 years ago
Closed 11 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•11 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•11 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Reporter | ||
Comment 2•11 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•11 years ago
|
||
Oh, and no, there's no need to specify the port anymore.
Assignee | ||
Comment 4•11 years ago
|
||
Updated with fixes
Attachment #771336 -
Attachment is obsolete: true
Attachment #771379 -
Flags: review?(wlachance)
Reporter | ||
Updated•11 years ago
|
Attachment #771379 -
Flags: review?(wlachance) → review+
Reporter | ||
Comment 5•11 years ago
|
||
Pushed: https://github.com/mozilla/mozbase/commit/582a2b3419ad53a4e9bc255b9e38b996c96cf8f8
Reporter | ||
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•