Skip to content

Most of the xml_patch unittests#14

Merged
elementgreen merged 4 commits intoKymorphia:masterfrom
dejlek:dejan/xml_path_unittests
Feb 16, 2026
Merged

Most of the xml_patch unittests#14
elementgreen merged 4 commits intoKymorphia:masterfrom
dejlek:dejan/xml_path_unittests

Conversation

@dejlek
Copy link
Copy Markdown
Contributor

@dejlek dejlek commented Feb 16, 2026

Aim is for near-full coverage.
There are 3-4 unittests that are failing and they need to be dealt separately (investigate why are they failing as they are support to pass).
Coverage of this module will be around 80% when we merge this.

patch.parseRenameCmd("root.node[][old_attr]"d, "new_attr"d);
patch.apply(tree, null);

assert(!("old_attr" in tree.root.children[0].attrs));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually use !in, but not that big of a deal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

auto patch = new XmlPatch();
bool caught = false;

try
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use assertThrown for this and others below? Example:

assertThrown!XmlPatchError(patch.parseSelector("node[=value]"d));

For cases where try/catch are used, I prefer to leave out the braces when there is only one statement in the try or catch blocks, to try and keep such code more compact.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@elementgreen elementgreen merged commit 1f247d8 into Kymorphia:master Feb 16, 2026
1 check passed
@dejlek dejlek deleted the dejan/xml_path_unittests branch February 16, 2026 19:26
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

Successfully merging this pull request may close these issues.

2 participants