cached_property attributes are called when importing library

[xiangfan2022]

RobotFramework incorrectly calls cached_property when importing a Python library using the cached_property method.

I found a previous issue on this topic, #4838, and it seems that the issue has been solved in 6.1.1. But in fact, the issue still exists. Please check the following steps, explanations, and suggestions.

Steps:

1. Python version 3.11.6
2. RobotFramework version is 6.1.1.
3. Create a python file named test.py as below, two methods decorated by cached_property, one will print some message, and the other will raise a RuntimeError.

from functools import cached_property

class TestCached:
    @cached_property
    def cached_property_with_print(self):
        print(f"This is in cached_property_with_print")

    @cached_property
    def cached_property_with_error(self):
        raise RuntimeError("This is in cached_property_with_error")

4. Create a robot file named test.robot as below, using the Import Library to import test.TestCached class.

*** Settings ***
Documentation       demo cached_property issue

*** Test Cases ***
1: import library with cached_property
    Import Library    test.TestCached

5. Run the test.robot and the console output is shown as below.

==============================================================================
Testcase
==============================================================================
Testcase.Test :: demo cached_property issue
==============================================================================
1: import library with cached_property                                This is in cached_property_with_print
[ WARN ] Imported library 'test.TestCached' contains no keywords.
1: import library with cached_property                                | PASS |
------------------------------------------------------------------------------
Testcase.Test :: demo cached_property issue                           | PASS |
1 test, 1 passed, 0 failed
==============================================================================
Testcase                                                              | PASS |
1 test, 1 passed, 0 failed
==============================================================================

6. From the console output, the cached_property_with_print was called, but there is no RuntimeError.(This is why issue Using cached properties in Python libraries breaks keyword discovery #4838 was treated as solved)

Explantions:

1. Check the source code of RobotFramework, file robot.running.testlibraries, line 340-354:

class _ClassLibrary(_BaseTestLibrary):

    def _get_handler_method(self, libinst, name):
        for item in (libinst,) + inspect.getmro(libinst.__class__):
            # `isroutine` is used before `getattr` to avoid calling properties.
            if (name in getattr(item, '__dict__', ())
                    and inspect.isroutine(item.__dict__[name])):
                try:
                    method = getattr(libinst, name)
                except Exception:
                    message, traceback = get_error_details()
                    raise DataError(f'Getting handler method failed: {message}',
                                    traceback)
                return self._validate_handler_method(method)
        raise DataError('Not a method or function.')

The condition if (name in getattr(item, '__dict__', ()) and inspect.isroutine(item.__dict__[name])) cannot filter out the cached_property method. The inspect.isroutine will give True for the cached_property instance. So the cached_property method will be called by line method = getattr(libinst, name). When this line met error, a DataError will be raised to it caller.

2. Check the source code of RobotFramework, file robot.running.testlibraries, line 271-276:

    def _try_to_get_handler_method(self, libcode, name):
        try:
            return self._get_handler_method(libcode, name)
        except DataError as err:
            self._adding_keyword_failed(name, err, self.get_handler_error_level)
            return None

When the error is raised from line return self._get_handler_method(libcode, name), then the line self._adding_keyword_failed(name, err, self.get_handler_error_level) will be executed. The _adding_keyword_failed will add error message to the syslog file (refer the syslog as below). So there is no error message shown in console output or log file. This is why issue #4838 is considered resolved. In that case, no error raised to console output mislead us.

3. check syslog:

20231027 22:53:04.080 | DEBUG | Started keyword 'BuiltIn.Import Library'.
20231027 22:53:04.087 | INFO  | Imported library class 'test.TestCached' from 'test.py'.
20231027 22:53:04.090 | INFO  | In library 'test.TestCached': Adding keyword 'cached_property_with_error' failed: Getting handler method failed: This is in cached_property_with_error
20231027 22:53:04.090 | DEBUG | Details:
Traceback (most recent call last):
  File "...functools.py", line 1001, in __get__
    val = self.func(instance)
          ^^^^^^^^^^^^^^^^^^^
  File "...test.py", line 11, in cached_property_with_error
    raise RuntimeError("This is in cached_property_with_error")
RuntimeError: This is in cached_property_with_error
20231027 22:53:04.090 | INFO  | In library 'test.TestCached': Adding keyword 'cached_property_with_print' failed: Not a method or function.
20231027 22:53:04.090 | INFO  | In library 'test.TestCached': Adding keyword 'property_with_print' failed: Not a method or function.

Suggestions:
For the file robot.running.testlibraries, line 346, and inspect.isroutine(item.__dict__[name])):, add one more condition and callable(item.__dict__[name]). This property and cached_property instance are not callable, so this condition can filter out them. Then the if condition would look like:

if (name in getattr(item, '__dict__', ())
    and inspect.isroutine(item.__dict__[name]) 
    and callable(item.__dict__[name])):

Hope this issue can be solved and help someone.

[pekkaklarck]

Thanks for a report and good example. I should have investigated #4838 in more detail and create a test for cached_property instead of trusting it to be a standard non-data descriptor that we already have tests for. More precisely, the problem is that our tests for non-data descriptors validate that possible errors accessing them are handled properly, when we shouldn't access cached_property at all. I don't know can we avoid accessing non-data descriptors in general, but cached_property is easy to special case.