Skip to content

Fix __add__: mutated arguments after operation#178

Open
miguelflor wants to merge 3 commits into
weiwei:masterfrom
miguelflor:master
Open

Fix __add__: mutated arguments after operation#178
miguelflor wants to merge 3 commits into
weiwei:masterfrom
miguelflor:master

Conversation

@miguelflor
Copy link
Copy Markdown

@miguelflor miguelflor commented May 12, 2026

This started with a issue #177 that I posted.
So I recommend reading it to understand the description of this PR.

Description

I found that after making the add on the Object JUnitXml the arguments would mutate, making the second add with the same arguments different. Which I believe is not desirable.

Problem 1

The behavior described in the issue happens because of the function .append(child) of etree.
The etree can come from two libraries as you can see form the snippet taken from the source code:

try:
    from lxml import etree
except ImportError:
    from xml.etree import ElementTree as etree

When a the function .append(child) is called and the child already as a parent, both libraries operate in different ways.

lxml

With this library when the .append(child) is trigger it moves each child from the original tree to the new tree.
That explains why in the code I shared in the PR, after the first + in the logger all the testsuites where moved to a new object and latter garage collected. When the +is called again both vars don't have any tests.

xml.etree

This is trickier.
the behavior is actually simpler just gives the child to the new parent, but the child will have two parents at the same time. Although I could not find any error I believe that this can be prone to error.

Problem 2

This problem arises when the arguments of the __add__ where considered under the function __eq__ that they had equal suites.
Because of the call _add_testcase_no_update_stats(case), it changes the suites making it with duplicate cases.
This error only happens when using xml.etree, and that's because the values where not move at first.

def add_testsuite(self, suite):                                   
    for existing_suite in self: 
        if existing_suite == suite: # Considered equal
            for case in suite:
                existing_suite._add_testcase_no_update_stats(case) # Adds to the suite permanently
            return

Test

The test made covers both problems at the same time (:

Fix

The fix is actually pretty simple, I just called deepcopy(suite) in the beginning of the function add_testsuite(self, suite).

Additional Notes

In the end a lot of yapping for a simple solution. If you have any questions feel free to ask.

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.

1 participant