Python monkeypatch an API and search through iterable object tree

Question

General information

I am using an acceptance testing framework (robot framework) and it has an API which allows you to 'visit' suites and tests before execution.

Specific goal

The framework does not save the original suites name (a.k.a. tests.my_test). If you wanted to run that test twice with different names it will throw out the tests component and become new_name.my_test. I wanted to be able to preserve that information so that while viewing it can have descriptive names, but while searching the suite trees I could still find the original test names and rerun.

Questions to people unfamiliar with the framework

I wanted to piggyback off of their work to stay as compatible with the framework as possible. The selection of rerunned tests is here. It includes a function gather_failed_tests which uses class GatherFailedTests. Since, that native feature of rerunfailed and my feature of rerunrenamedtests can't coexist I chose to simply monkeypatch the GatherFailedTests class to inherit from my class, which fixes the names before it would gather the tests.

Is that a dangerous idea anyway? Should I copy all the source for the function gather_failed_tests and rename them in their entirety? It's a maintainability vs stability problem. I either can monkey patch a minimal component and hope I get any fixes added to the function or surrounding framework or I can have to manually update the whole chain.

I feel like there is a more efficient way to go through the iterable suites. suite_obj.suites returns an iterable of suites. However the initial top level suite itself also needs to be searched. Is there a way to collapse config_suites and config_all_suites simply?

It is ultimately a tree of suites and I need to handle the first element in each branch that has the originallongname in metadata.

Questions for anyone who knows robot

I'm unhappy with wiping out the parent when I am configuring a non-root suite test.parent.parent = None. I'm uncertain if why that method doesn't just configure the parent's criticality instead of failing, but regardless of how it functions is there a better way to alter the longname property without destroying info?

from robot.api import SuiteVisitor
from robot.conf import gatherfailed


class rename(SuiteVisitor):
    """Store the original longname so that rerunning can happen even with virtually reorganized tests."""

    def __init__(self, new_name):
        self.new_name = new_name

    def start_suite(self, suite):
        originallongname = suite.longname
        suite.metadata.setdefault('originallongname', originallongname)
        suite.configure(name=self.new_name)


class resetname(SuiteVisitor):
    def config_test(self, suite):
        originallongname = suite.metadata['originallongname']
        try:
            suite.configure(name=originallongname)
        # Critically setting from non-root error
        except TypeError:
            for test in suite.tests:
                test.parent.name = originallongname
                test.parent.parent = None

    def config_all_suites(self, suite):
        for suite in suite.suites:
            try:
                self.config_test(suite)
            except KeyError:
                self.config_suites(suite)

    def config_suites(self, suite):
        try:
            self.config_test(suite)
        except KeyError:
            self.config_all_suites(suite)

    def start_suite(self, suite):
        self.config_suites(suite)


class RenameThenGatherFailedTests(resetname):

    def __init__(self):
        self.tests = []

    def visit_test(self, test):
        if not test.passed:
            self.tests.append(test.longname)

    def visit_keyword(self, kw):
        pass

gatherfailed.GatherFailedTests = RenameThenGatherFailedTests


class rerunrenamedtests(SuiteVisitor):
    def __init__(self, output):
        self.output = output

    def start_suite(self, suite):
        tests = gatherfailed.gather_failed_tests(self.output)
        suite.filter(included_tests=tests)

If it would help to see how this would be used you could see here.

Answer 1

I think it is more futureproof to use multi-inheritance to resetname and use GatherFailedTests together rather than copy and pasting the code and only having it have single inheritance with copy pasted source.

class RenameThenGatherFailedTests(resetname, gatherfailed.GatherFailedTests):
    pass


gatherfailed.GatherFailedTests = RenameThenGatherFailedTests

I think this is simply better than the copy some code and don't copy other hybrid in the original question. However, I'm still uncertain if simply copying and altering the entire source would be a better practice.