[Django] #25170: assertXMLEqual fails when white space is present outside XML fragment
Django
-----------------------------------+--------------------
Reporter: shelldweller | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.8
Severity: Normal | Keywords: test
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+--------------------
Currently this test fails:
{{{#!python
class MyTestCase(TestCase):
def test_xml(self):
self.assertXMLEqual('<greeting>hello</greeting>',
'<greeting>hello</greeting>\n')
}}}
I believe this is a bug because white space outside root tag should not
matter. It makes testing rather confusing when reading XML documents from
files (which may or may not have trailing new line characters).
--
Ticket URL: <https://code.djangoproject.com/ticket/25170>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: shelldweller | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* version: 1.8 => master
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Seems reasonable as the docstring says "Whitespace in most cases is
ignored".
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:1>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by quamilek):
* owner: nobody => quamilek
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:2>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by mishunika):
At the time of analyzing the issue, I didn't notice that the issue is
already assigned. Anyway, just because the fix seems so simple, and there
is no pull request yet, I published my own solution as a PR.
The fixed branch is available here:
https://github.com/mishunika/django/tree/ticket_25170
And the PR is here: https://github.com/django/django/pull/5049
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:3>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by quamilek):
I started working on this bug after assigned.
This is PR for this issue: https://github.com/django/django/pull/5052
Resolution of this bug is different then @mishunika.
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:4>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
I think [https://github.com/django/django/pull/5052 PR 5052] is more
correct in that it will continue to consider whitespace *inside* a tag
significant; e.g. the following snippets shouldn't be considered equal:
{{{
<tag>foo </tag>
<tag>foo</tag>
}}}
Anyone to confirm that? The author of
[https://github.com/django/django/pull/5049 PR 5049] argues otherwise.
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:5>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:6>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:7>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by jbzdak):
Whitespace around tags is indeed siginificant --- technically following
documents *are* different
{{{
<foo></foo>
}}}
and
{{{
<foo>
</foo>
}}}
however for most practical purposes this difference is insignificant, as
most of XML-based formats ignore whitespace altogether (like html).
Despite that I think that in unittesting framework we should rather assume
pessimistic case that whitespace does matter.
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:8>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by nostalgiaz):
According to https://tools.ietf.org/html/rfc3470#section-4.16
> In XML instances all white space is considered significant and is by
default visible to processing applications
So, I think that @jbzdak is right.
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:9>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by nostalgiaz):
* needs_better_patch: 1 => 0
Comment:
The fixed branch is available here:
https://github.com/nostalgiaz/django/tree/pull_25170
And the PR is here: https://github.com/django/django/pull/5580
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:10>
Django
Reporter: shelldweller | Owner: quamilek
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by charettes):
* stage: Accepted => Ready for checkin
Comment:
Contained white-space should really be considered significant in the HTML
case.
If I remember correctly it affect `display: inline` elements in some way
(or it did in the past).
Stripping leading and trailing white space makes sense to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:11>
Django
Reporter: shelldweller | Owner: quamilek
Component: Testing framework | Version: master
Keywords: test | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"2085d8d5bc2625076983d3be14b30ea684c3f0d5" 2085d8d5]:
{{{
#!CommitTicketReference repository=""
revision="2085d8d5bc2625076983d3be14b30ea684c3f0d5"
Fixed #25170 -- Made assertXMLEqual()/assertXMLNotEqual() ignore leading
and trailing whitespace.
Thanks Jacek Bzdak for indepdently contributing a similar fix.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25170#comment:12>