[Django] #31250: assertXMLEqual fails on processing instructions
Django
---------------------------------------------+------------------------
Reporter: Matt Fisher | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 3.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
Similar bug to https://code.djangoproject.com/ticket/30497, so I've based
this report on that one.
Prepare Django project:
{{{
$ python -m venv env
$ . ./env/bin/activate
$ pip install django
$ django-admin startproject p1
$ cd p1
}}}
Create p1/tests.py, with xml taken from [https://www.w3.org/TR/xml-
model/#d0e679 W3 xml-model] and [https://www.w3.org/TR/2010/REC-xml-
stylesheet-20101028/ W3 xml-stylesheet] examples:
{{{
from django.test import TestCase
class MyTestCase(TestCase):
def test_assert_xml_equal_xml_model(self):
xml1 = '''
<?xml version="1.0"?>
<?xml-model
href="http://www.docbook.org/xml/5.0/rng/docbook.rng"?>
<?xml-model
href="http://www.docbook.org/xml/5.0/xsd/docbook.xsd"?>
<book xmlns="http://docbook.org/ns/docbook">
</book>
'''
self.assertXMLEqual(xml1, xml1)
def test_assert_xml_equal_stylesheet(self):
xml = '''
<?xml-stylesheet href="common.css"?>
<?xml-stylesheet href="default.css" title="Default style"?>
<?xml-stylesheet alternate="yes" href="alt.css"
title="Alternative style"?>
<?xml-stylesheet href="single-col.css" media="all and (max-
width: 30em)"?>
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>Example with xml-stylesheet processing
instructions</title>
</head>
<body>
</body>
</html>
'''
self.assertXMLEqual(xml, xml)
}}}
Run the tests:
{{{
$ ./manage.py test
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
FF
======================================================================
FAIL: test_assert_xml_equal_stylesheet (tests.MyTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File ".../dev/django/env/lib/python3.7/site-
packages/django/test/testcases.py", line 843, in assertXMLEqual
result = compare_xml(xml1, xml2)
File ".../dev/django/env/lib/python3.7/site-
packages/django/test/utils.py", line 596, in compare_xml
return check_element(want_root, got_root)
AttributeError: 'ProcessingInstruction' object has no attribute 'tagName'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File ".../dev/django/p1/tests.py", line 28, in
test_assert_xml_equal_stylesheet
self.assertXMLEqual(xml, xml)
File ".../dev/django/env/lib/python3.7/site-
packages/django/test/testcases.py", line 846, in assertXMLEqual
self.fail(self._formatMessage(msg, standardMsg))
AssertionError: First or second argument is not valid XML
'ProcessingInstruction' object has no attribute 'tagName'
======================================================================
FAIL: test_assert_xml_equal_xml_model (tests.MyTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File ".../dev/django/env/lib/python3.7/site-
packages/django/test/testcases.py", line 843, in assertXMLEqual
result = compare_xml(xml1, xml2)
File ".../dev/django/env/lib/python3.7/site-
packages/django/test/utils.py", line 596, in compare_xml
return check_element(want_root, got_root)
AttributeError: 'ProcessingInstruction' object has no attribute 'tagName'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File ".../dev/django/p1/tests.py", line 12, in
test_assert_xml_equal_xml_model
self.assertXMLEqual(xml1, xml1)
File ".../dev/django/env/lib/python3.7/site-
packages/django/test/testcases.py", line 846, in assertXMLEqual
self.fail(self._formatMessage(msg, standardMsg))
AssertionError: First or second argument is not valid XML
'ProcessingInstruction' object has no attribute 'tagName'
----------------------------------------------------------------------
Ran 2 tests in 0.007s
FAILED (failures=2)
Destroying test database for alias 'default'...
}}}
As discussed in [https://github.com/django/django/pull/11402 the PR for
the previous fix], assertXMLEqual probably shouldn't compare what's
outside the root, so ignoring `Node.PROCESSING_INSTRUCTION_NODE` in
[https://github.com/django/django/blob/3.0.3/django/test/utils.py#L577-L580
first_node] would probably do the job (tests pass after this change).
--
Ticket URL: <https://code.djangoproject.com/ticket/31250>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-----------------------------------+------------------------------------
Reporter: Matt Fisher | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-----------------------------------+------------------------------------
Changes (by felixxm):
* version: 3.0 => master
* easy: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/31250#comment:1>
Django
Reporter: Matt Fisher | Owner: Yuri Savin
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-----------------------------------+--------------------------------------
Changes (by Yuri Savin):
* owner: nobody => Yuri Savin
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31250#comment:2>
Django
-----------------------------------+--------------------------------------
Reporter: Matt Fisher | Owner: Yuri Savin
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-----------------------------------+--------------------------------------
Comment (by Yuri Savin):
Write PR for this ticket https://github.com/django/django/pull/12446
Added a check for the availability of fields in the node. The original
branch was passed nodes COMMENT_NODE and DOCUMENT_TYPE_NODE. I left pass
only COMMENT_NODE
--
Ticket URL: <https://code.djangoproject.com/ticket/31250#comment:3>
Django
Reporter: Matt Fisher | Owner: Yuri
| Savin
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Yuri Savin):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31250#comment:4>
Django
Reporter: Matt Fisher | Owner: Yuri Savin
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-----------------------------------+--------------------------------------
* has_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
Yuri, you shouldn't mark you own patches as "Ready for checkin".
--
Ticket URL: <https://code.djangoproject.com/ticket/31250#comment:5>
Django
-----------------------------------+--------------------------------------
Reporter: Matt Fisher | Owner: Yuri Savin
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-----------------------------------+--------------------------------------
Comment (by Yuri Savin):
Replying to [comment:5 felixxm]:
> Yuri, you shouldn't mark you own patches as "Ready for checkin".
Oh, I'm sorry. I'm trying to notify you that the patch is ready
--
Ticket URL: <https://code.djangoproject.com/ticket/31250#comment:6>
Django
-----------------------------------+--------------------------------------
Reporter: Matt Fisher | Owner: Yuri Savin
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-----------------------------------+--------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31250#comment:7>
Django
Reporter: Matt Fisher | Owner: Yuri
| Savin
Type: Bug | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31250#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Matt Fisher | Owner: Yuri
| Savin
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"54b7af7eb4107f933d14da0737b539bbe38d0fc8" 54b7af7e]:
{{{
#!CommitTicketReference repository=""
revision="54b7af7eb4107f933d14da0737b539bbe38d0fc8"
Fixed #31250 -- Ignored processing instructions in
assertXMLEqual()/assertXMLNotEqual().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31250#comment:9>