data performance keywords#458
Conversation
Dor-bl
left a comment
There was a problem hiding this comment.
Looks like a solid addition! I have a few suggestions mostly around type hinting and docstrings:
1. Type Hinting for Automatic Conversion
In get_performance_data, the data_read_timeout argument is parsed manually using int(data_read_timeout). Since Robot Framework 4.0+, you can use Python type hints to let Robot Framework handle argument conversion automatically.
from typing import Optional
def get_performance_data(self, package_name: str, data_type: str, data_read_timeout: Optional[int] = None):Using this approach, if a user passes 3 as a string in their Robot Framework test, it will automatically be converted to an integer, and you won't need the explicit int() cast.
2. Error Handling for the Timeout Casting
If type hinting isn't adopted, int(data_read_timeout) could throw a ValueError if the user passes a non-numeric string. The current check does handle "" if they pass it exactly, but using type hints would make it more robust.
3. Misleading Argument Name (data_read_timeout)
The argument is named data_read_timeout, but the docstring describes it as: Number of attempts to read.
While this naming comes directly from the underlying Appium Python Client method, it might be confusing for end users who expect "timeout" to be a time value. I suggest adding a small clarification note in the docstring to emphasize that this represents retries/attempts rather than time.
4. Docstring Consistency
In get_performance_data_types, the docstring uses Return: instead of Returns: which is slightly inconsistent with the library's typical Sphinx/reST format seen in other methods.
Example Refactored Version:
def get_performance_data(self, package_name: str, data_type: str, data_read_timeout: Optional[int] = None):
"""Returns performance data for the given ``package_name`` and ``data_type``.\n
*Android only.* See `Get Performance Data Types` for supported types.
Args:
- ``package_name`` (str): The application package name
- ``data_type`` (str): One of the supported performance data types
- ``data_read_timeout`` (int, optional): The number of attempts to read the data (Note: despite the name, this is retry attempts, not a time duration).
Returns:
Any: Performance data as returned by Appium (structure depends on driver)
"""
driver = self._current_application()
if data_read_timeout is None:
return driver.get_performance_data(package_name, data_type)
return driver.get_performance_data(package_name, data_type, data_read_timeout)
Implements