regression: changeset e909ceae9b3e72b72e0a2baaa92bba9714f18cd2 breaks some tests for Windows with sqlite

[Michael Manfre]

Regression added with changeset e909ceae9b3e72b72e0a2baaa92bba9714f18cd2.

python tests\runtests.py --settings=test_sqlite admin_scripts.tests.StartProject.test_custom_project_template_from_tarball_by_url


======================================================================
FAIL: test_custom_project_template_from_tarball_by_url (admin_scripts.tests.StartProject)
Make sure the startproject management command is able to use a different project template from a tarball via a url
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\projects\django\django\tests\admin_scripts\tests.py", line 1605, in test_custom_project_template_from_tarball_by_url
    self.assertNoOutput(err)
  File "C:\projects\django\django\tests\admin_scripts\tests.py", line 182, in assertNoOutput
    self.assertEqual(len(stream), 0, "Stream should be empty: actually contains '%s'" % stream)
AssertionError: Stream should be empty: actually contains 'Traceback (most recent call last):
  File "C:\projects\django\django\django\bin\django-admin.py", line 5, in <module>
    management.execute_from_command_line()
  File "C:\projects\django\django\django\core\management\__init__.py", line 397, in execute_from_command_line
    utility.execute()
  File "C:\projects\django\django\django\core\management\__init__.py", line 390, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "C:\projects\django\django\django\core\management\base.py", line 242, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "C:\projects\django\django\django\core\management\base.py", line 289, in execute
    output = self.handle(*args, **options)
  File "C:\projects\django\django\django\core\management\commands\startproject.py", line 31, in handle
    super(Command, self).handle('project', project_name, target, **options)
  File "C:\projects\django\django\django\core\management\templates.py", line 125, in handle
    base_subdir)
  File "C:\projects\django\django\django\core\management\templates.py", line 208, in handle_template
    return self.extract(absolute_path)
  File "C:\projects\django\django\django\core\management\templates.py", line 302, in extract
    archive.extract(filename, tempdir)
  File "C:\projects\django\django\django\utils\archive.py", line 49, in extract
    with Archive(path) as archive:
  File "C:\projects\django\django\django\utils\archive.py", line 58, in __init__
    self._archive = self._archive_cls(file)(file)
  File "C:\projects\django\django\django\utils\archive.py", line 138, in __init__
    self._archive = tarfile.open(file)
  File "C:\Python27\Lib\tarfile.py", line 1665, in open
    raise ReadError("file could not be opened successfully")
tarfile.ReadError: file could not be opened successfully
'

e909ceae9b3e72b72e0a2baaa92bba9714f18cd2 is the first bad commit
commit e909ceae9b3e72b72e0a2baaa92bba9714f18cd2
Author: Ramiro Morales <cramm0@gmail.com>
Date:   Sat Jun 1 14:24:46 2013 -0300

    Made django.test.testcases not depend on staticfiles contrib app.

    Do this by introducing a django.contrib.staticfiles.testing.StaticLiveServerCase
    unittest TestCase subclass.

    Fixes #20739.

:040000 040000 3602788911b0c9f9d6c95805b321968cfed98571 11b64417027fd4776f0123e3c0022d4155aea4b9 M      django
:040000 040000 0699494f0516bb71df49f1d88ab3b3a096ddd33c 3e238009f30bc5223ac6711c560f041af8949fa7 M      docs
:040000 040000 47ae44d2101b694c1d570fa29a483de2e43cdd54 c341edbef9d7c04bbf5760cf4a652152eba62e59 M      tests

[Michael Manfre]

Same changeset also responsible for servers.tests.LiveServerViews.test_no_collectstatic_emulation failure.

======================================================================
FAIL: test_no_collectstatic_emulation (servers.tests.LiveServerViews)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\projects\django\django\tests\servers\tests.py", line 148, in test_no_collectstatic_emulation
    self.assertEqual(err.code, 404, 'Expected 404 response')
AssertionError: Expected 404 response

[Ramiro Morales]

Original committer here.

This solves both problems:

diff --git a/django/test/testcases.py b/django/test/testcases.py
index a657530..64a80ab 100644
--- a/django/test/testcases.py
+++ b/django/test/testcases.py
@@ -958,7 +958,8 @@ class FSFilesHandler(WSGIHandler):

     def serve(self, request):
         os_rel_path = self.file_path(request.path)
-        final_rel_path = posixpath.normpath(unquote(os_rel_path)).lstrip('/')
+        os_rel_path = posixpath.normpath(unquote(os_rel_path))
+        final_rel_path = os_rel_path.replace('\\', '/').lstrip('/')
         return serve(request, final_rel_path, document_root=self.get_base_dir()

     def __call__(self, environ, start_response):

We have historic, deeper inconsistencies, namely:

1. The file_path() method of contrib.staticfiles StaticFilesHandler middleware (and of the django.test.testcases static and media ones introduced by my commit too because they are copied from it) changes the request path to one with OS-specific path separators (e.g. '/a/b/c.jpg' to '\a\b\c.jpg' on Windows.)
2. That OS-specific path is passed to the respective serve() method where it is, among other things, massaged with posixpath.normpath() (note how it has no effect on Windows) and then handed to a relevant serve() view (django.contrib.staticfiles.views.serve() and django.views.static.serve() respectively).
3. Then the staticfiles serve() view invokes that app's finders machinery which consumes OS-specific paths and returns forward-slash delimited paths. The newly-introduced middlewares don't do this and this is where things diverged because all further processing is done on a OS-specific path and as such most of the processing performed by django.views.static.serve() has no effect because it expects a '/'-delimited path.

The patch above changes this by using a hack similar to one used by django.views.static.serve() to replace '\' with '/' to restore behavior parity.

A longer term project would be to fix (to always consume '/'-delimited paths and so make it possible to use that kind of paths through all the above described workflow and leave handling of OS-specific ones to the finders), formalize and document the (internal, so far undocumented) API with staticfiles finders backends, although it might break third party backends.

[Michael Manfre]

The patch fixes the test failures I reported.

[Florian Apolloner]

Why not just use .lstrip(os.separator) instead of the replacing?

[Ramiro Morales]

Replying to apollo13:

Why not just use .lstrip(os.separator) instead of the replacing?

Because at that point _all_ the path path separators in 'os_rel_path' are platform-specific not just the leading one.

The lstrip() was alredy there (see also e.g. django.contrib.staticfiles.views.serve()), it's the the replacement of path separators (that needs to happen before) what is new . I've just refactored the code a bit.

[Ramiro Morales <cramm0@…>]

In 783620ccc827057d1d0f2c6f4808350be9193bf9:

Fixed #21162 -- Better emulation of staticfiles middleware.

Code had been added in e909ceae9b. Solves test suite failures observed
on Windows.

Thanks Michael Manfre for the report. Refs #20739.