Skip to content

Pytest fixture to change actors restart_limit doesn't work as expected #41

Closed

Description

What happened?

Originally created by @ela-kotulska-frequenz

We added fixture to change the restart_limit in actors, but we didn't enable it for all actors in unit tests.

It looks like if we use this fixture, we can't pass any arguments to the tests functions.

Example:

https://github.com/frequenz-floss/frequenz-sdk-python/blob/20e1dffadcb728443190e1dcfae45d68545ba962/tests/configs/test_config_manager.py

If class TestActorConfigManager: derives after (IsolatedAsyncioTestCase) then the tests fails with error:

self = <tests.mos_framework.configs.test_config_manager.TestActorConfigManager testMethod=test_update>
func = <bound method TestActorConfigManager.test_update of <tests.mos_framework.configs.test_config_manager.TestActorConfigManager testMethod=test_update>>
args = (), kwargs = {}

    def _callMaybeAsync(self, func, /, *args, **kwargs):
        assert self._asyncioTestLoop is not None
>       ret = func(*args, **kwargs)
E       TypeError: test_update() missing 1 required positional argument: 'config_file'

What did you expect instead?

The restart_limit is overridden correctly.

Affected version(s)

All

Affected part(s)

Unit, integration and performance tests (part:tests)

Extra information

Fixture to change restart_limit:

@pytest.fixture(scope="session", autouse=True)
def disable_actor_auto_restart(): # type: ignore
"""Disable auto-restart of actors while running tests.
Since this is auto-use, the yield part (and restore of the variable) is not
strictly needed, but we leave it as an example.
Note: Test class must derive after unittest.IsolatedAsyncioTestCase.
Otherwise this fixture won't run.
"""
original_restart_limit = decorator.BaseActor.restart_limit
decorator.BaseActor.restart_limit = 0
yield
decorator.BaseActor.restart_limit = original_restart_limit

Activity

  1. added
    part:testsAffects the unit, integration and performance (benchmarks) tests
    on Oct 19, 2022
  2. leandro-lucarella-frequenz commented on Oct 19, 2022

    @leandro-lucarella-frequenz
    ContributorAuthor

    Comment by @jakub-toptal

    Looks like the IsolatedAsyncioTestCase class doesn't properly pick up pytest fixtures. IsolatedAsyncioTestCase does expose some methods like setUp and tearDown so if we really needed, we should be able to create a fixture ourselves there instead of using pytest. The module you mentioned, doesn't need to inherit from IsolatedAsyncioTestCase, but if it did, we could move the logic for creating a tmp config file into setUp method, save it as an instance attribute, e.g. self.config_file, then use that in the test method, and finally clean it up in tearDown.

  3. added 3 commits that reference this issue on Dec 2, 2022
    61382a3
    aa5b9e4
    e048f4e
  4. Repository owner moved this from To do to Done in Python SDK Roadmapon Dec 5, 2022
  5. added a commit that references this issue on Apr 26, 2023
    2087fff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    part:testsAffects the unit, integration and performance (benchmarks) testspriority:lowThis should be addressed only if there is nothing else on the tabletype:bugSomething isn't working

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions