Time parsing is incorrect

[asaf-kali]

In my company, we run tests in parallel.

Pytest exports the test suite with a (correct) time tag:

<?xml version="1.0" encoding="utf-8"?>
<testsuites>
    <testsuite name="pytest" errors="0" failures="14" skipped="0" tests="76" time="4005.821" timestamp="...." hostname="....">
        <testcase classname="...." name="...." time="661.025"/>

Indeed, tests took about an hour.

The time provided by TestSuite.time was 23406 seconds, which is the accumeltated time of all the tests.
It seems the logic in TestSuite.update_statistics does not take into account such cases, so I suggest the following logic change:
If time tag already exists on the element, skip the sum logic, and use the tag value as value for time.

[EnricoMi]

Why would you be interested in the wall-clock time, as it depends on the parallelism of the test execution? What is generally more useful is the CPU time used by the tests to identify regression.

I think you can always implement your own method and extract time from the JUnitXml instance and have access to both metrics.

[asaf-kali]

In my usecase, I want to present how long (clock wall) test execution took - it's an important metric for us.
My point is that there is a piece of information that is present in the original report, and is ignored by the parser. I didn't find a way to access it myself (by extending the JUnitXml).
The behaviour I would expect is:

1. Rename the existing time property to something like accumelated_time (to emphasize the fact that its the sum of the time the tests in the suite took).
2. Create a time field that just yields the original time field in the suite.

LMK what you think. If you have a simple way of extracting the time without this refactoring, please let me know. Thanks 🙏🏼

[EnricoMi]

What if we allow controlling whether update_statistics is being called during loading file or not through an argument? Wiring this through might not be easy, though.

[Cube707]

I don't quite get where the problem lies.

Running the following code: (note that tests/data/pytest_success.xml is a recent addition to master and not jetz present in the v3.2.0 tag)

from junitparser import JUnitXml

junit = JUnitXml.fromfile("tests/data/pytest_success.xml")


for suite in junit:
    print(f"{suite.time=}")
    for testcase in suite:
        print(f"{testcase.time=}")


print("\n-- updating statistics")
junit.update_statistics()
for suite in junit:
    print(f"{suite.time=}")
    for testcase in suite:
        print(f"{testcase.time=}")

you will note that suite.time is different from the sum of all testcase.time and only after manually calling update_statistics() the described problem occurs. This is behavior is identical on current master and v3.2.0.

And I believe that updating suite.time with the sum of all test-times is a reasonable default, as update_statistics() is expected to be called after adding/deleting/moddifying testcases. But I agree that this could be made configurable via an option.

[EnricoMi]

I agree with @asaf-kali that we should distinguish between the sum of the inner times (what update_statistics returns) and the time a test suite is annotated with (which might be the sum or simply the wall clock time).

Giving user access to any of these numbers explicitly would eliminate any issue.

I'd suggest:

1. Have TestSuite.time return the time that exists in the file, as this is a better source of truth. This might be considered a breaking change or a bug fix.
2. Provide TestSuite.time_accumulated or TestSuite.test_case_time, computed through update_statistics(). Have TestSuite.time return TestSuite.time_accumulated if no suite time is set.