Skip to content

Stop using atexit and add integration tests for tmpdir fixture #10545

Open
@ykadowak

Description

@ykadowak

What's the problem this feature will solve?

This makes it possible to add integration tests for tmpdir directory cleanup process.

Currently, tests of tmpdir directory cleanup is insufficient in a way that it does not have any integration tests such as a test to make sure if there remains only N directories after executing a lot of test runs.

Now that we have retention_policy and retention_count configuration, these tests are more and more important.

Describe the solution you'd like

Stop using atexit for removing locks and directories. Instead, introduce a class that does similar job as atexit but can execute registered functions whenever you want. Use the new class to register those functions and execute them on pytest_sessionfinish.

The reason why there isn't such integration tests is it's impossible because atexit is used to cleanup directories.
Since atexit executes functions on program exit, test process can't capture the outcome.

So why is atexit currently used?

There are two atexits in tmpdir related code.

  1. Removing directory lock
    return register(cleanup_on_exit)
  2. Removing directory itself
    atexit.register(

Seems like atexit for removing directory lock has been there since the early days of Pytest. So I couldn't find a reasoning.

On the other hand, atexit for removing directories was introduced to fix this issue(#1120). As far as I understood, the reason why atexit was introduced to solve this issue is because atexit is used to remove the lock. So if we could stop using atexit for removing the lock, we could stop using atexit at all. And I think it's okay to do it since the longest lifetime of a tmpdir is the end of a session.

Alternative Solutions

For testing, we can mitigate the current situation by mocking atexit.register and spying the call. But mocking is way less practical than real testing.

Additional context

Made a PR for PoC #10546.
With this change, I also made sure that it still works correctly with this parallel #1120 (comment) condition.

Metadata

Metadata

Assignees

No one assigned

    Labels

    plugin: tmpdirrelated to the tmpdir builtin plugintopic: fixturesanything involving fixtures directly or indirectlytype: enhancementnew feature or API change, should be merged into features branch

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions