Deprecate assertEqualXMLStructure()

[sebastianbergmann]

I have never understood the use case for this assertion and have never seen it used in the wild.

[povetina]

That actually doesn't mean people don't use it.
For example, I maintain a project which among other things exports XML data to remote server.
To check that my code generates correct XML file I use precisely assertEqualXMLStructure() function.
Also, it doesn't strike me as the best practice to remove the function completely without providing a new one.

[tim-bezhashvyly]

Also using it. Now have to work it around. 😔

[tshafer]

I am using it as well.

[theseer]

I'm not sure whether or not this very method or its implementation are the best way to solve the use case it at least for me did cover.

But the general use case is actually rather simple, given that the definition of "equal" with XML documents isn't exactly clear:

<?xml version="1.0" ?>
<root>
    <node />
</root>

./.

<?xml version="1.0" ?>
<root><node /></root>

Are these equal?

If you parse an XML document into a DOM and compare the resulting object graph, they won't be because of additional DOMText-Elements representing the whitespaces for the indentation and the line breaks in the first XML.

In many (most?) contexts though, the whitespace is actually pretty irrelevant. So how would one assert that two structures are "otherwise" equal?

Maybe the method name doesn't (didn't) really transport that intention, but that's what at least I have been using it for.

[theseer]

Another example:

<?xml version="1.0" ?>
<root a="true" b="true" />

./.

<?xml version="1.0" ?>
<root b="true" a="true" />

Logically (semantically?), the two XML structures are equal. On a string and DOM-Basis, they are not.

[theseer]

Talked with @sebastianbergmann, and we decided that I'll re-implement this functionality.

A PR will be provided for it hopefully soon, not sure yet as to when exactly I'll have time to do though.

[theseer]

The PR mentioned re-implements the main aspect of the original, deprecated assertion assertEqualXMLStructure: The comparison of two XML trees for structural identicality.

Two DOM trees are to be considered identical when, after canonicalization, the following holds true:

• The same child - parent relationship is maintained
• The same attributes are set on a node
• The attribute values are identical
• The textual content of nodes are identical

Looking at the options of the original, deprecated method, this is stricter. And maybe not actually "structural". The original method for instance ignored namespace defining attributes as well as textual content.

If we want to have a mere "grid" without actually looking at the values / text content, this can of course also be implemented - but should be a different method. As I didn't see a use case for that though, I did not reimplement that aspect yet.

My implementation in the PR basically ignores any whitespace and, due to canonicalization, reorders attributes as needed so the order of attributes is not relevant.

The name assertDOMTreesEqualStructurally is subject to change, only pending a better naming suggestion. It should probably reflect the actual assertion better. It might be kept if the "grid" alike implementation is wanted.

[theseer]

I spent some time searching how people use this method (https://github.com/search?q=assertEqualXMLStructure&type=Code) and am by now quite convinced that most people, me included, used the method wrong.

I used it for templado's engine tests and the fact attribute values as well as text content are actually ignored seems to be a hardly known fact. For me, it was unexpected and quite some of my tests where actually only working by accident.

So for that reason alone, it's good this method is deprecated. I currently believe we should offer various alternatives to replace it though:

• A method that does what I implemented in my PR (the current method name though is not fitting)
• A method to assert that, like the original method, two elements are structurally eqal but find a better, clearly describing name and a more sane implementation
• A method to assert a DOMDocument adheres to an XSD

Opinions?

[RafaelKr]

I was just wondering why this method was deprecated and landed here. Will the implementation from @theseer be implemented in a future version?

[theseer]

Possible, even though I'm not fully convinced yet I like the implementation. One of the reasons why it's still in draft.

We at the very least do have to find a better name, as the current name doesn't properly reflect what it's asserting for.

[RafaelKr]

I had a look at your implementation and noticed this is actually not what I would need. As you really check for identical documentElements a viable function name could be assertIdenticalDOM or assertIdenticalDOMTree. What do you think?

My current use case is that I deserialize XML from a message queue into a PHP Object and then serialize the object back to XML and the output should have the identical parent-child relationship and the elements should have the same attributes. In my case the values are not important. For example it's possible that the Date formats are different. But the elements and attributes should be the same.

So I think a general function which would be helpful would be a function which can:

1. Check if only parent-child tree is identical
2. Check that all attribute keys are the same
3. Check if all attribute keys and attribute values are the same
4. Check if element values are the same

By default I think 1 should always be checked, and the others are incremental.

So maybe a bitmask could be used. A rough POC could look like this:

class DomAssert {
    // 0b00000001
    const EQUAL_ATTRIBUTE_KEYS = 1 << 0;

    // 0b00000010
    const EQUAL_ATTRIBUTE_KEYS_AND_VALUES = 1 << 1;

    // 0b00000100
    const EQUAL_VALUES = 1 << 2;

    // 0b00001000
    const IGNORE_VALUE_WHITESPACES = 1 << 3;

    public static function assertEqualDOMStructure(\DOMElement $expectedElement, \DOMElement $actualElement, int $comparisonLevel) {
        $expectedDocument = new DOMDocument();
        $expectedDocument->appendChild($expectedDocument->importNode($expectedElement, true));
        $expectedElementXmlString = $expectedDocument->C14N(false);

        $actualDocument = new DOMDocument();
        $actualDocument->appendChild($actualDocument->importNode($actualElement, true));
        $actualElementXmlString = $actualDocument->C14N(false);

        if ($comparisonLevel & static::IGNORE_VALUE_WHITESPACES) {
            // not sure if this actually trims the whitespaces in the node values?
            $expectedDocument->preserveWhiteSpace = false;
            $actualDocument->preserveWhiteSpace = false;
        }

        $expectedDocument->loadXML($expectedElementXmlString);
        $actualDocument->loadXML($actualElementXmlString);

        static::assertEqualDOMStructureRecursive(
            $expectedDocument->documentElement,
            $actualDocument->documentElement,
            $comparisonLevel
        );

    }

    public static function assertEqualDOMStructureRecursive(\DOMElement $expectedElement, \DOMElement $actualElement, int $comparisonLevel) {
        // compare current element tagName

        if ($comparisonLevel & static::EQUAL_ATTRIBUTE_KEYS || $comparisonLevel & static::EQUAL_ATTRIBUTE_KEYS_AND_VALUES) {
            // check attribute keys

            if ($comparisonLevel & static::EQUAL_ATTRIBUTE_KEYS_AND_VALUES) {
                // check the attribute value
            }
        }

        if ($comparisonLevel & static::EQUAL_VALUES) {
            // compare element values
        }

//        for (...) {
//            static::assertEqualDOMStructureRecursive($expectedChildElement, $actualChildElement, $comparisonLevel);
//        }
    }
}

$domAssert = new DomAssert();

// expected element
$ee = new DOMElement();
// actual element
$ae = new DOMElement();

DomAssert::assertEqualDOMStructure($ee, $ae,DomAssert::EQUAL_ATTRIBUTE_KEYS);
DomAssert::assertEqualDOMStructure($ee, $ae,DomAssert::EQUAL_ATTRIBUTE_KEYS_AND_VALUES);
DomAssert::assertEqualDOMStructure($ee, $ae,DomAssert::EQUAL_ATTRIBUTE_KEYS | DomAssert::EQUAL_ATTRIBUTE_KEYS_AND_VALUES); // same as previous line
DomAssert::assertEqualDOMStructure($ee, $ae,DomAssert::EQUAL_VALUES);
DomAssert::assertEqualDOMStructure($ee, $ae,DomAssert::EQUAL_ATTRIBUTE_KEYS_AND_VALUES | DomAssert::EQUAL_VALUES);
DomAssert::assertEqualDOMStructure($ee, $ae,DomAssert::EQUAL_ATTRIBUTE_KEYS | DomAssert::EQUAL_VALUES);

[theseer]

I think it becomes more and more obvious the decision to deprecate the original method was a good idea ;)

Regarding your PoC: We have various use cases here merged into one solution. I'm not yet sure we'd need or even want that. Let me think about that a bit more and maybe speak to @sebastianbergmann tomorrow ;)