Reporting test results as JSON and XML

Question

I have two functions now with very similar functionality, and would like to merge them into one more generic function. The only difference between those two functions is that one handles json input file and the other xml input file. I could use if statements to achieve this, but I find it cumbersome, other suggestions would be much appreciated!

def update_json_type_report(self, testcase_id, did_any_test_fail, test_case_steps):
    print "Adding results to test case id: %s" % testcase_id

    fail_or_pass = '(/) ' # initialize test to pass

    expected_result = {
        "result":""
    }

    for i, step in enumerate(test_case_steps):
        for tests in self.parsed_report_file["suites"]:
            for test in tests["tests"]:
                tmp_result = ""
                if test["name"] == test_case_steps[i]['step']['raw'] and test["state"]:
                    if "error" in test:
                        fail_or_pass = '(x) '
                        did_any_test_fail = 3
                        tmp_result += fail_or_pass + test["error"] + '\n'
                    else:
                        # can have many tests per step, append them all in one string adding '\n' at end of each line
                        tmp_result += fail_or_pass + test["name"] + '\n'

                    break
            #import pdb; pdb.set_trace()
            expected_result['result'] = tmp_result
            if tmp_result: 
                self.jira_obj.send_put('xray', 'test/%s/steps/%s' % (testcase_id, test_case_steps[i]['id']), expected_result)

    return did_any_test_fail

def update_xml_type_report(self, testcase_id, did_any_test_fail, test_case_steps):
    print "Adding results to test case id: %s" % testcase_id

    fail_or_pass = '(/) ' # initialize test to pass

    expected_result = {
        "result":""
    }

    for i, step in enumerate(test_case_steps):
        for xml_testsuite in self.parsed_report_file:
            for xml_testcase in xml_testsuite.iter('testcase'):
                tmp_result = ""
                if xml_testcase.attrib["name"] == test_case_steps[i]['step']['raw'] and "hook" not in xml_testcase.attrib["name"]:
                    if xml_testcase.find('system-err') is not None:
                        fail_or_pass = '(x) '
                        did_any_test_fail = 3
                        tmp_result += fail_or_pass + xml_testcase.find('system-err').text + '\n'
                    else:
                        # can have many tests per step, append them all in one string adding '\n' at end of each line
                        tmp_result += fail_or_pass + xml_testcase.attrib["name"] + '\n'

                    break

            expected_result['result'] = tmp_result
            if tmp_result: 
                self.jira_obj.send_put('xray', 'test/%s/steps/%s' % (testcase_id, test_case_steps[i]['id']), expected_result)

    return did_any_test_fail

Answer 1

I based this off of your own answer. There are still some things to improve:

• This is Python, you don't need to "define" a variable before assigning to it, in this case tests.

• The ...iter('testcase') seems weird, do you just want a copy of the list or something? Otherwise you could just assign the value itself or use list(...):

  tests = list(self.parsed_report_file.iter('testcase'))
  # or even just
  tests = self.parsed_report_file.iter('testcase')
  

• The step from enumerate(test_case_steps) isn't being used, why not? In fact I don't see any reason for the enumerate given that the index is just used to get the step value again:

  for step in test_case_steps:
      ...
      for test in tests:
          ...
          if self.is_json_report_file and test["name"] == step['step']['raw'] and test["state"]:
              ...
  

• The two if blocks share code and the conditions are duplicated.

• The variable expected_result isn't very useful, just direct construct the to-be-passed-in value, that's clearer:

  self.jira_obj.send_put('xray', 'test/%s/steps/%s' % (testcase_id, step['id']), {"result": expected_result})
  

• It doesn't look like more than one of the tmp_result += ... calls can be run each loop, so just use assignment, again, that's much clearer. I'd prefer to use None for a default case here, but then again, as long as you know that the empty string is "false" that's fine.
• fail_or_pass isn't reset in each loop step, is that correct? It's also always the prefix for the tmp_result variable, so you can delay the formatting until much later, same with the newline.
• did_any_test_fail = 3 is obscure. A simple yes/no (True/False) is a better choice for a boolean return value.

All in all that looks like this now:

def update_test_case_expected_result(self, testcase_id):
    did_any_test_fail = False # initialize to pass
    fail_or_pass = '(/) ' # initialize test to pass

    if self.is_json_report_file:
        tests = [test for tests in self.parsed_report_file["suites"] for test in tests["tests"]]
    else:
        tests = list(self.parsed_report_file.iter('testcase'))

    for step in self.get_test_case_steps(testcase_id):
        for test in tests:
            tmp_result = ""
            if test["name"] == step['step']['raw']:
                if self.is_json_report_file:
                    if test["state"]:
                        if "error" in test:
                            fail_or_pass = '(x) '
                            did_any_test_fail = True
                            tmp_result = test["error"]
                        else:
                            # can have many tests per step, append them all in one string adding '\n' at end of each line
                            tmp_result = test["name"]
                else if "hook" not in test.attrib["name"]:
                    if test.find('system-err') is not None:
                        fail_or_pass = '(x) '
                        did_any_test_fail = True
                        tmp_result = test.find('system-err').text
                    else:
                        # can have many tests per step, append them all in one string adding '\n' at end of each line
                        tmp_result = test.attrib["name"]

            if tmp_result:
                self.jira_obj.send_put('xray', 'test/%s/steps/%s' % (testcase_id, step['id']), {"result": fail_or_pass + tmp_result + "\n"})

    return did_any_test_fail

Answer 2

It looks like you need to abstract out how you get the element that contains the test cases and then how you determine errors from the nodes in the element.

for test in self.parsed_report_file["suites"]["tests"]:

and

for xml_testcase in xml_testsuite.iter('testcase'):

get you the parent element to start the loop and then you need to determine whether the items are in error. I think it would make things easier if you did some of the refinement when you get the initial set of elements rather than doing more filtering in the loops. Since in the JSON case you only care about nodes matching test["name"] == test_case_steps[i]['step']['raw'] and test["state"], do that filtering when you grab the parent. Same for the XML, can you use XPath or similar to filter down to xml_testcase.attrib["name"] == test_case_steps[i]['step']['raw'] and "hook" not in xml_testcase.attrib["name"] before you start?

Once you have all that, the Jira-specific handling of errors (self.jira_obj.send_put) can be abstracted out to its own function that accepts the id, test step id and result object.

Answer 3

@Tom, building on your suggestions, The first two loops are constructed outside the main loop, now the code is much cleaner and much fewer lines. Here is what I came up with, I'm eager to know if code can be even more simplified!

def update_test_case_expected_result(self, testcase_id):
    test_case_steps = self.get_test_case_steps(testcase_id)

    did_any_test_fail = 0 # initialize to pass
    fail_or_pass = '(/) ' # initialize test to pass

    expected_result = {
        "result":""
    }

    tests = None
    if self.is_json_report_file:
        tests = [test for tests in self.parsed_report_file["suites"] for test in tests["tests"]]
    else:
        tests = [value for value in self.parsed_report_file.iter('testcase')]

    for i, step in enumerate(test_case_steps):
        for test in tests:
            tmp_result = ""
            if self.is_json_report_file and test["name"] == test_case_steps[i]['step']['raw'] and test["state"]:
                if "error" in test:
                    fail_or_pass = '(x) '
                    did_any_test_fail = 3
                    tmp_result += fail_or_pass + test["error"] + '\n'
                else:
                    # can have many tests per step, append them all in one string adding '\n' at end of each line
                    tmp_result += fail_or_pass + test["name"] + '\n'

            if not self.is_json_report_file and test.attrib["name"] == test_case_steps[i]['step']['raw'] and "hook" not in test.attrib["name"]:
                if test.find('system-err') is not None:
                    fail_or_pass = '(x) '
                    did_any_test_fail = 3
                    tmp_result += fail_or_pass + test.find('system-err').text + '\n'
                else:
                    # can have many tests per step, append them all in one string adding '\n' at end of each line
                    tmp_result += fail_or_pass + test.attrib["name"] + '\n'

            expected_result['result'] = tmp_result
            if tmp_result: 
                self.jira_obj.send_put('xray', 'test/%s/steps/%s' % (testcase_id, test_case_steps[i]['id']), expected_result)

    return did_any_test_fail