Merge lp://qastaging/~coreygoldberg/selenium-simple-test/selftest-server-port2 into lp://qastaging/selenium-simple-test
Proposed by
Corey Goldberg
| Status: | Merged |
|---|---|
| Approved by: | Vincent Ladeuil |
| Approved revision: | 379 |
| Merged at revision: | 373 |
| Proposed branch: | lp://qastaging/~coreygoldberg/selenium-simple-test/selftest-server-port2 |
| Merge into: | lp://qastaging/selenium-simple-test |
| Diff against target: |
338 lines (+114/-40) 9 files modified
src/sst/__init__.py (+2/-0) src/sst/actions.py (+2/-2) src/sst/scripts/run.py (+23/-14) src/sst/selftests/assert_urls.py (+11/-11) src/sst/selftests/current_url.py (+5/-4) src/sst/selftests/html5.py (+2/-1) src/sst/selftests/title.py (+9/-8) src/sst/tests/__init__.py (+16/-0) src/sst/tests/test_django_devserver.py (+44/-0) |
| To merge this branch: | bzr merge lp://qastaging/~coreygoldberg/selenium-simple-test/selftest-server-port2 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Corey Goldberg (community) | Approve | ||
| Vincent Ladeuil (community) | Approve | ||
|
Review via email:
|
|||
Commit message
default django devserver port changed to 8120 and better error handling
Description of the change
default port is now: 8120
if port is in use, it quits with less cryptic "port is in use" error.
To post a comment you must log in.
Revision history for this message
| Corey Goldberg (coreygoldberg) | # |
review:
Approve
Revision history for this message
| Corey Goldberg (coreygoldberg) wrote | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
Could you please use DEVSERVERPORT everywhere instead of using the hardcoded 8120 value ? Otherwise changing that value in sst/__init__.py won't make the tests pass.
While there, I think we would even want to use an URL instead of just the port so that we won't have to redo the same work if something similar happen with the hostname.
80 + for desc, cmd, arg in cleanups:
This forces all users to add a needless 'None' parameter when using cleanups while not allowing several parameters to be used if needed. I would rather use cmd(*args) below:
88 + cmd(arg)
this will also clarify the proposal by avoiding the added parameter for code not directly related to this fix.
204 +def check_devserver
205 + """check if port is ok to use for django devserver"""
This relies on django using the same SO_REUSEADDR trick, I think it's worth a comment.
Alternatively, you could check that the django server itself is properly running by checking a specific url defined for that purpose in our test application.