0
\$\begingroup\$

This is an automation code written in Python, code is initiated with robotscript. This function validates that my input list matches the list that the program provides me with. I need to do typecasting because robotframework only allows my to insert parameters as unicode strings, typecast is also needed so I can check values within range.

I want to know if this code can be better modified, I'm doing a lot of iteration through list and nested lists. Any comments will be appreciated.

def validate_estimates(expect_coordinates):
    """ Validates all Estimates lines that are drawn on display.
        input_param: expect_coordinates, which is a list of lists for validation
        Values that are collected from the UI need to be typecasted to either time or float
            for range check validation.
        Function, throws error ir sublists for input and actual values do not match as well if
            values from expected lists are not within range.
    """
    scene = squishtest.waitForObject("{type='isds::MainView' unnamed='1' visible='1' windowTitle='EdgeView'}")
    expect_coordinates_list = ast.literal_eval(expect_coordinates)
    measurement_values_list = []
    first_iter_special_case = True
    for si in squishtest.object.children(scene):
        if squishtest.className(si) == 'QGraphicsItem':
            if si.type == 65536:
                for ssi in squishtest.object.children(si):
                    for sssi in squishtest.object.children(ssi):
                        measurement_values = []
                        try:
                            qoo = squishtest.castToQObject(ssi)
                        except Exception, e:
                            raise AssertionError("Error in typecast: ", e)

                        if first_iter_special_case:
                            for i in range(4):
                                if i == 0:
                                    measurement_values.append(datetime.datetime.strptime(str(qoo.measurementField), '%H:%M'))  # time value
                                elif i == 1 or i == 2:
                                    nm_str = str(qoo.measurementField)
                                    measurement_values.append(float(nm_str[:nm_str.find(' ')]))
                                elif i == 3:
                                    eet_str = str(qoo.measurementField)
                                    eet_str = eet_str[:eet_str.find(' ')]
                                    measurement_values.append(datetime.datetime.strptime(eet_str, '%H:%M'))  # time value
                                squishtest.mouseClick(squishtest.waitForObject(sssi))
                            first_iter_special_case = False
                        else:
                            for i in range(3):
                                if i == 0:
                                    measurement_values.append(datetime.datetime.strptime(str(qoo.measurementField), '%H:%M'))  # time value
                                elif i == 1:
                                    nm_str = str(qoo.measurementField)
                                    measurement_values.append(float(nm_str[:nm_str.find(' ')]))
                                elif i == 2:
                                    eet_str = str(qoo.measurementField)
                                    eet_str = eet_str[:eet_str.find(' ')]
                                    measurement_values.append(datetime.datetime.strptime(eet_str, '%H:%M'))  # time value
                                squishtest.mouseClick(squishtest.waitForObject(sssi))            
                    measurement_values_list.append(measurement_values) # append sublists to list

    # list lengths validations
    if len(measurement_values_list) != len(expect_coordinates_list):
        raise AssertionError('Error list lenghts: Validation list must be of length[%s] not length[%s]' % (len(measurement_values_list), len(expect_coordinates_list)))
    for i in range(len(measurement_values_list)):
        if len(measurement_values_list[i]) != len(expect_coordinates_list[i]):
            raise AssertionError('Error sublist lengths: validation sublist number[%s] is of length[%s] should be of length[%s]' % (i+1, len(expect_coordinates_list[i]), len(measurement_values_list[i])))

    # validation of values from isds and our expected values
    for i in range(len(measurement_values_list)):
        for j in range(len(measurement_values_list[i])):
            if type(measurement_values_list[i][j]) is datetime.datetime:
                utility_time_in_range(measurement_values_list[i][j], expect_coordinates_list[i][j])
            elif isinstance(measurement_values_list[i][j], float): 
                if not (measurement_values_list[i][j]-30 <= float(expect_coordinates_list[i][j]) <= measurement_values_list[i][j]+30):
                    raise AssertionError('Error given value[%s] outsite lower boundaries[%s] and uper boundaries[%s]' % (expect_coordinates_list[i][j], measurement_values_list[i][j]-30, measurement_values_list[i][j]+30))


def utility_time_in_range(actual_time, expected_time):
    """ Utility function
        input_param: actual_time and expected_time.
        Verifies that expected_time is within boundraries, otherwise throws out error.
    """
    et = datetime.datetime.strptime(expected_time, '%H:%M')
    lower_time_boundaries = actual_time - datetime.timedelta(minutes=10)
    higher_time_boundaries = actual_time + datetime.timedelta(minutes=10)
    if not (lower_time_boundaries <= et <= higher_time_boundaries):
        raise AssertionError('Error in time: Given value[%s] outsite lower boundaries[%s] or uper boundaries[%s]' % (expected_time, lower_time_boundaries, higher_time_boundaries))
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

The first thing is that the first function is way too long. In general either try to reduce the amount of work, or split it into smaller chunks early.

Next, the nesting is also quite deep in the first loop. IMO one of the best ways to tackle that is to continue instead of nesting too many ifs.

  • Use of magic constants. What is 65536 supposed to mean?
  • Variable names. si/ssi/sssi doesn't ring a bell for me. Might be okay for you; I'd still use something more descriptive.

qoo can be moved up one for-loop.

The for i in range(...): if i == 0 ... seems really weird. If it's just done to reuse the wait method, I'd rather create a helper function for that and deal with the once duplicated step separately. It's way clearer if not done with this idiom.

  • The appending to measurement_values_list has the wrong indentation? It should probably append the currently assembled list instead of only once at the end.

  • Instead of using for i in range(len(..)) it's generally better to use either normal iteration over the elements, enumerate, or something like zip/izip.

I can't tell if the if-else blocks with the type tests should have an alternative clause for the "error" case; I'd maybe go with a normal else and assert that the type is e.g. float if that was meant.

  • Don't recompute values. If in doubt assign it to a meaningful name and reuse. E.g. the x - 30/x + 30 parts, or lengths and so on.

  • In general use instanceof instead of type(...) is. Don't know if it was intended this way though.

All in all it would probably look like this (note that x to w should be replaced with either better names or moved into smaller helper functions; same for some_step and wait_for_sssi):

def gather_measurement_values_list():
    measurement_values_list = []
    first_iter_special_case = True

    for si in squishtest.object.children(scene):
        if squishtest.className(si) != 'QGraphicsItem':
            continue

        if si.type != 65536:
            continue

        for ssi in squishtest.object.children(si):
            try:
                qoo = squishtest.castToQObject(ssi)
            except Exception, e:
                raise AssertionError("Error in typecast: ", e)

            for sssi in squishtest.object.children(ssi):
                measurement_values = []

                def wait_for_sssi():
                    squishtest.mouseClick(squishtest.waitForObject(sssi))

                measurement_values.append(datetime.datetime.strptime(str(qoo.measurementField), '%H:%M'))  # time value
                wait_for_sssi()

                def some_step():
                    nm_str = str(qoo.measurementField)
                    measurement_values.append(float(nm_str[:nm_str.find(' ')]))
                    wait_for_sssi()

                some_step()

                # repeat previous step once more
                if first_iter_special_case:
                    first_iter_special_case = False
                    some_step()

                eet_str = str(qoo.measurementField)
                eet_str = eet_str[:eet_str.find(' ')]
                measurement_values.append(datetime.datetime.strptime(eet_str, '%H:%M'))  # time value
                wait_for_sssi()

                measurement_values_list.append(measurement_values) # append sublists to list

    return measurement_values_list


def validate_estimates(expect_coordinates):
    """ Validates all Estimates lines that are drawn on display.
        input_param: expect_coordinates, which is a list of lists for validation
        Values that are collected from the UI need to be typecasted to either time or float
            for range check validation.
        Function, throws error ir sublists for input and actual values do not match as well if
            values from expected lists are not within range.
    """
    scene = squishtest.waitForObject("{type='isds::MainView' unnamed='1' visible='1' windowTitle='EdgeView'}")
    expect_coordinates_list = ast.literal_eval(expect_coordinates)
    measurement_values_list = gather_measurement_values_list()

    # list lengths validations
    if len(measurement_values_list) != len(expect_coordinates_list):
        raise AssertionError('Error list lengths: Validation list must be of length[%s] not length[%s]' % (len(measurement_values_list), len(expect_coordinates_list)))

    for i, (x, y) in enumerate(zip(measurement_values_list, expect_coordinates_list)):
        if len(x) != len(y):
            raise AssertionError('Error sublist lengths: validation sublist number[%s] is of length[%s] should be of length[%s]' % (i+1, len(x), len(y)))

    # validation of values from isds and our expected values
    for i, (x, y) in enumerate(zip(measurement_values_list, expect_coordinates_list)):
        for j, (v, w) in enumerate(zip(x, y)):
            if type(v) is datetime.datetime:
                utility_time_in_range(v, w)
            elif isinstance(v, float):
                if not (v - 30 <= float(w) <= v + 30):
                    raise AssertionError('Error given value[%s] outsite lower boundaries[%s] and uper boundaries[%s]' % (w, v - 30, v + 30))


def utility_time_in_range(actual_time, expected_time):
    """ Utility function
        input_param: actual_time and expected_time.
        Verifies that expected_time is within boundraries, otherwise throws out error.
    """
    et = datetime.datetime.strptime(expected_time, '%H:%M')
    lower_time_boundaries = actual_time - datetime.timedelta(minutes=10)
    higher_time_boundaries = actual_time + datetime.timedelta(minutes=10)
    if not (lower_time_boundaries <= et <= higher_time_boundaries):
        raise AssertionError('Error in time: Given value[%s] outsite lower boundaries[%s] or uper boundaries[%s]' % (expected_time, lower_time_boundaries, higher_time_boundaries))
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.