Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reimplement main aspect of assertEqualXMLStructure #4507

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

theseer
Copy link
Collaborator

@theseer theseer commented Nov 1, 2020

As discussed in #4091, there is a valid use case for having an assertion ensuring two DOM sub trees are structurally identical.

This PR re-implements the main aspect of the original, deprecated 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 appears to be stricter. The original method for instance ignored namespace defining attributes as well as textual content.

As I didn't see a use case for that, I did not reimplement that aspect. My implementation 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.

@codecov
Copy link

codecov bot commented Nov 1, 2020

Codecov Report

Attention: Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.

Project coverage is 83.59%. Comparing base (61180a8) to head (85d4c05).
Report is 7140 commits behind head on main.

Files with missing lines Patch % Lines
src/Framework/Assert.php 0.00% 14 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #4507      +/-   ##
============================================
- Coverage     83.69%   83.59%   -0.10%     
- Complexity     4663     4664       +1     
============================================
  Files           293      293              
  Lines         12030    12044      +14     
============================================
  Hits          10068    10068              
- Misses         1962     1976      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sebastianbergmann sebastianbergmann added feature/assertion Issues related to assertions and expectations type/enhancement A new idea that should be implemented labels Nov 1, 2020
@sebastianbergmann sebastianbergmann added this to the PHPUnit 9.5 milestone Nov 1, 2020
@sebastianbergmann
Copy link
Owner

This branch has conflicts that must be resolved.

@WinterSilence
Copy link

you use troll's solutions..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/assertion Issues related to assertions and expectations type/enhancement A new idea that should be implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants