#28212 closed Cleanup/optimization (fixed)
Allow customizing the port that LiveServerTestCase uses
| Reported by: | Robert Rollins | Owned by: | nobody |
|---|---|---|---|
| Component: | Testing framework | Version: | 1.11 |
| Severity: | Normal | Keywords: | |
| Cc: | Simon Charette | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
As of Django 1.11, I cannot see any way for selenium tests to work in a Docker environment. Binding LiveServerTestCase's WSGIServer to port 0 makes it impossible for your host image's Dockerfile to expose the necessary port for a Selenium server to be able to connect to your host.
I'd like to propose the attached patch to remedy this problem. It leaves the default behavior the same as it currently stands, but allows the developer to override that behavior as needed by simply defining the port property in their test class.
Attachments (2)
Change History (12)
by , 8 years ago
| Attachment: | allow_assigning_port.patch added |
|---|
comment:1 by , 8 years ago
| Patch needs improvement: | set |
|---|---|
| Summary: | The port 0 change for LiveServerTestCase breaks Selenium testing in Docker → Allow customizing the port that LiveServerTestCase uses |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Bug → Cleanup/optimization |
comment:4 by , 8 years ago
Hmm, never written a test for django before. How should I go about that?
I suggest you add a test method to tests.servers.tests. LiveServerPort which finds an available port, set it as an attribute to dynamically a create LiveServerBase subclass (look at test_port_bind for that), call setUpClass() on it and assert TestClass.live_server_url contains the specified port.
comment:5 by , 8 years ago
Well that was much less painful than I expected! I've had some pretty harrowing experiences trying to set up and run other app's test suites in the past, but Django's was a breeze!
Here's a new patch with the code change and a test I wrote. Unfortunately, I don't really know what to do about exceptions thrown during the test (like what
LiveServerPort.test_port_bind does), so the test code isn't complete. But I think it exercises the relevant code changes, at least.
by , 8 years ago
| Attachment: | allow_assigning_port-with-test.patch added |
|---|
comment:6 by , 8 years ago
| Cc: | added |
|---|
Awesome work Robert,
Glad to hear it was easy to get the test suite running on your machine!
Would it be possible for you to submit your patch as Github pull request? It would allow the CI infrastructure to run the full test suite against your patch and assert everything is fine. This will also give the patch more visibility and make the review easier.
comment:7 by , 8 years ago
Sure thing! The reason I didn't do so initially was that I didn't know if I should submit the PR against the stable/1.11.x branch, or against master (which appears to be whole hog into Django 2.0 mode, so I'm not even sure my change would apply cleanly). I'll try it as a commit against stable/1.11.x, and see what happens.
comment:8 by , 8 years ago
OK, I opened a PR for this patch @ https://github.com/django/django/pull/8534
I think it would be nice to have a test for that, perhaps with some mocking. The 1.11 release notes could also be amended to mention this, with a note that it's added in 1.11.2.