Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#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)

allow_assigning_port.patch (2.2 KB ) - added by Robert Rollins 8 years ago.
allow_assigning_port-with-test.patch (3.7 KB ) - added by Robert Rollins 8 years ago.

Download all attachments as: .zip

Change History (12)

by Robert Rollins, 8 years ago

Attachment: allow_assigning_port.patch added

comment:1 by Tim Graham, 8 years ago

Patch needs improvement: set
Summary: The port 0 change for LiveServerTestCase breaks Selenium testing in DockerAllow customizing the port that LiveServerTestCase uses
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

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.

comment:2 by Robert Rollins, 8 years ago

Hmm, never written a test for django before. How should I go about that?

comment:3 by Tim Graham, 8 years ago

The contributing docs for unit tests should help.

comment:4 by Simon Charette, 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 Robert Rollins, 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 Robert Rollins, 8 years ago

comment:6 by Simon Charette, 8 years ago

Cc: Simon Charette 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 Robert Rollins, 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 Robert Rollins, 8 years ago

OK, I opened a PR for this patch @ https://github.com/django/django/pull/8534

comment:9 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: newclosed

In 877d7b7:

[1.11.x] Fixed #28212 -- Allowed customizing the port that LiveServerTestCase uses.

comment:10 by Tim Graham <timograham@…>, 7 years ago

In b6d4b6e:

Fixed #28212 -- Allowed customizing the port that LiveServerTestCase uses.

Forwardport of 877d7b71ae952b3bc946e5187d6c23039a71614d from stable/1.11.x

Note: See TracTickets for help on using tickets.
Back to Top