Opened 9 years ago
Closed 4 years ago
#26591 closed Bug (fixed)
Incorrect Manifest Keys for ManifestStaticFilesStorage on Windows
Reported by: | David Sanders | Owned by: | nobody |
---|---|---|---|
Component: | contrib.staticfiles | Version: | dev |
Severity: | Normal | Keywords: | staticfiles manifest manifeststaticfileststorage windows |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The manifest created by ManifestStaticFilesStorage
on Windows has some incorrect keys (haven't determined the pattern yet) where slashes are '\\'
instead of '/'.
This silently 'succeeds' because HashedFilesMixin
will hash the original file on a 'cache miss' (in the case of ManifestStaticFilesStorage
it's a manifest miss) and insert the result into the in-memory hashed files map.
I've attached a patch for staticfiles_tests which checks that the manifest is the same in memory and on disk after the manifest tests finish (unless it was a test which cleared the in-memory version). It passes for the Linux build but fails on the Windows build, nicely illustrating the problem.
Attachments (1)
Change History (4)
by , 9 years ago
Attachment: | manifeststaticfilesstorage_test.patch added |
---|
comment:1 by , 9 years ago
Description: | modified (diff) |
---|---|
Triage Stage: | Unreviewed → Accepted |
The final version of the patch shouldn't put the assertions in tearDown()
, if possible.
comment:2 by , 9 years ago
Yea, it's not a 'correct' place to put assertions, but it was the best spot to put the assertion to run the code after all test cases, short of a much more intrusive patch. The nice bit about putting it there is that it automatically is run for all new test cases added.
The alternative would be to add it to the end of each of the test cases (I think it's a couple dozen), or to think of a clever way to get a 'post-test pre-tearDown' hook in place.
comment:3 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fixed in 53bffe8d03f01bd3214a5404998cb965fb28cd0b. The attached test case became assertPostCondition()
. The author explained in PR comments finding, reporting, and fixing the issue in the same PR.
Test coverage patch