gurock / trcli

TR CLI (trcli) is a command line tool for interacting with TestRail.
Mozilla Public License 2.0
48 stars 39 forks source link

[XML] Tests cases with multiple test ids present gets copied #162

Open jazerix opened 9 months ago

jazerix commented 9 months ago

In testcases where multiple properties has the test_id name, only the last value is persisted, which results in only one test being submitted and the previous ones being ignored.

A problematic xml file could be structured as such:

<testcase>
    <!-- other stuff --> 
    <properties>
    <property name="test_id" value="C576" />
    <property name="test_id" value="C577" />
    <property name="test_id" value="C566" />
    </properties>
</testcase>

When a JUnit XML file is ingested by the cli, it is parsed by the parse_file method found within readers/junit_xml.py. During parsing, each element is iterated over. The parser attempt to extract the test_id and then and saves it to a global variable case_id, this has the downside that when the next element containing a test_id name is read, it effectively overwrites the current case_id.

Issue being resolved: https://github.com/gurock/trcli/issues/130

Solution description

To solve this issue, I've introduced a global copies variable that "follows" the current case, such that it is automatically reset each time a new test case is being parsed. During parsing the parser now checks if the case_id is not None (indicating that the case_id was set by anothertest_id). In this event, thetest_idis added to thecopies` array.

When the test case is done being parsed and is added to the list of tests cases (test_cases variable), the copies are iterated over and each create a deep copy of the current test_case instance, alter their respective case_id and appends it to the list.

This results in each test_id becoming an independent test and submitted to TestRail as such.

Changes

Altered readers/junit_xml.py:

During parsing, now checks if the case id is already set:

parsed_case_id = int(prop.value.lower().replace("c", "")) 
if case_id is None:
    case_id = parsed_case_id 
else:
    copies.append(parsed_case_id)

Added copy_coplicate_test_cases method:

def copy_duplicate_test_cases(self, copies, test_cases):
        """Creates a deep copy of the current test case for each test_id present"""
        for case_id in copies:
            case = copy.deepcopy(test_cases[len(test_cases)-1])
            case.case_id = case_id
            case.result.case_id = case_id
            test_cases.append(case)

Which is called when the parser is done parsing the case.

Potential impacts

The code added should not have any impact as it does alter the flow under the documented scenarioes (i.e. where one test case has one test_id).

All tests also pass.

Steps to test

Create an a junit test file where one testcase contains more than one test_id property.

PR Tasks

jazerix commented 9 months ago

I have added a test that validates the behavior described in the related issue and fixed in this PR.

I would also like to add, that I do not deem this to be the best solution. A better approach could be to allow for multiple case_ids within the TestRailCase class - this would however require a substantial overhead to refactor the codebase. Magic methods could be used for backwards compatibility.

Alas, this solution is quite simple has the benefit that it does not affect the remaining codebase nor the logic herein :smile:

Fishbowler commented 1 month ago

In the absence of staff reviewing, and having encountered the problem myself, I thought I'd have a go at reviewing the PR in hopes of expediting its inclusion.

I'm not sure that this fully resolves #130. That ticket sites the example title= test_case_1 [C123 C456 C789] This PR solves for XML with multiple automation_ids, but doesn't include name parsing, which is called in L120 of your modified junit_xml.py and would need to be modified to return multiple matches rather than a single (int,string).

HTH