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

Unexpected diff #11

Open
donmendelson opened this issue Dec 15, 2021 · 1 comment
Open

Unexpected diff #11

donmendelson opened this issue Dec 15, 2021 · 1 comment

Comments

@donmendelson
Copy link
Member

One test case reported an unexpected difference where elements were identical but in a different order. The current code failed to determine the elements that were the same.

The current logic to compare elements is as follows:

  1. Compare the element names.
  2. If the element names are the same, compare the values of "id" attribute if present.
  3. If the elements are the same and "id" is not present, compare the values of "name" attribute if present.

An order difference is detected if any of the above conditions report an unequal result. Only id and name attributes are considered keys; all other attributes are considered non-key.

In the example, the elements names were all the same but they contain neither "id" nor "name" attributes. Therefore, they are all considered equal from an ordering perspective. The "value" attribute and element text are thus reported as changes of elements that seemingly have not moved.

@donmendelson
Copy link
Member Author

To correct this problem, a fourth step is added to element comparison: to compare the element text. This resolves the test case. That is, cases where the element name and text are the same, but the elements lack id or name attributes. However, it has the effect of changing the analysis of another case, where the elements are the same but only the text has changed. Before, this was reported as a replace of the text. Now is is reported as an element remove + add. This is considered a beneficial trade-off. Identical elements now correctly do not report a diff, while the other case is still reported as a diff but in a different way.

donmendelson added a commit that referenced this issue Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant