Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#26755 closed Cleanup/optimization (fixed)

test_middleware_classes_headers fails when django files aren't writable

Reported by: Raphaël Hertzog Owned by: nobody
Component: Core (Other) Version: 1.9
Severity: Normal Keywords:
Cc: Chris Lamb Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

commit abc0777b63057e2ff97eee2ff184356051e14c47 broke the test suite Debian runs against the installed package:
https://ci.debian.net/data/packages/unstable/amd64/p/python-django/20160610_144236.autopkgtest.log.gz

The problem is that the setUp() method tries to create a file in the django/conf/project_template/project_name/ directory but that directory is owned by root:root in the installed Debian package.
The test suite should not assume that it can write to the tree hosting the django code.

A possible fix would be to use some import magic:

with open(template_settings_py) as f:
    m = imp.load_module('settings', f, template_settings_py, ('', '', imp.PY_SOURCE))

(I can't figure out the equivalent with the importlib module)

Or to copy the file to a temporary directory that you add to sys.path.

Change History (10)

comment:1 by Tim Graham, 8 years ago

Component: UncategorizedCore (Other)
Easy pickings: unset
Summary: test_middleware_classes_headers is failing when django files are not writabletest_middleware_classes_headers fails when django files aren't writable
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

comment:3 by Claude Paroz, 8 years ago

Patch needs improvement: set

Thanks for the patch, but adding a new usage of a Python deprecated module (imp) is not an option.
Would it be possible to explore the other option (copy the file to a temporary directory that you add to sys.path)? We even have a utility (django.test.utils.extend_sys_path) for that.

comment:4 by Chris Lamb, 8 years ago

Patch needs improvement: unset

Thanks. Updated PR/patch.

comment:5 by Claude Paroz, 8 years ago

Triage Stage: AcceptedReady for checkin

Looks good!

comment:6 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: newclosed

In 2ec56bb7:

Fixed #26755 -- Fixed test_middleware_classes_headers if Django source isn't writable.

comment:7 by Chris Lamb, 8 years ago

Could this be backported to 1.11.x?

comment:8 by Tim Graham, 8 years ago

Yes, if you provide a Python 2 compatible version (tempfile.TemporaryDirectory is new in Python 3.2).

in reply to:  8 comment:9 by Chris Lamb, 8 years ago

Replying to Tim Graham:

Yes, if you provide a Python 2 compatible version (tempfile.TemporaryDirectory is new in Python 3.2).

Even in spite of https://github.com/django/django/blob/stable/1.11.x/tests/project_template/test_settings.py#L12 ? :)

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

In fa63fc91:

[1.11.x] Fixed #26755 -- Fixed test_middleware_classes_headers if Django source isn't writable.

Backport of 2ec56bb78237ebf58494d7a7f3262482399f0be6 from master

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