Use tempfile consistently across all tests

[osanseviero]

Is your feature request related to a problem? Please describe. At the moment, some tests use tempfile while others use tearDown to remove the tests

Describe the solution you'd like

Use tempfile everywhere. Examples

• https://github.com/huggingface/huggingface_hub/blob/main/tests/test_snapshot_download.py#L72
• https://github.com/huggingface/huggingface_hub/blob/main/tests/test_hf_api.py#L458

Additional context Some files that should be checked

• https://github.com/huggingface/huggingface_hub/blob/main/tests/test_file_download.py (downloads files but does not delete them)
• HfLargefilesTest https://github.com/huggingface/huggingface_hub/blob/main/tests/test_hf_api.py#L1155
• torch HubMixingTest https://github.com/huggingface/huggingface_hub/blob/main/tests/test_hubmixin.py#L64
• keras HubMixingTestKeras https://github.com/huggingface/huggingface_hub/blob/main/tests/test_keras_integration.py#L75
• https://github.com/huggingface/huggingface_hub/blob/main/tests/test_repocard.py#L105
• https://github.com/huggingface/huggingface_hub/blob/main/tests/test_repository.py#L76
• https://github.com/huggingface/huggingface_hub/blob/main/tests/test_repository.py#L1591
• https://github.com/huggingface/huggingface_hub/blob/main/tests/test_snapshot_download.py#L34

cc @adrinjalali @LysandreJik

[artemisep]

Hi, I could look into this. Thanks!

[artemisep]

I looked into the use of tempfile vs tearDown in https://github.com/huggingface/huggingface_hub/blob/main/tests/test_snapshot_download.py#L34 tempfile was doing it's job right, create temp folder/files and delete upon exit on the other hand, the tearDown() function was used to remove the folder/files that was copied to the local working directory where pytest was run, when a repository instance is initiated in L38. https://github.com/huggingface/huggingface_hub/blob/main/tests/test_snapshot_download.py#L38 Is this an intended behaviour? Thanks!

[adrinjalali]

Thanks for looking into this @Ruihua-Fang . The test you are referring to does not have the intended behavior. What we would like to have is to use either the tempfile module from python (https://docs.python.org/3/library/tempfile.html) or when appropriate the temp file fixtures from pytest (https://docs.pytest.org/en/6.2.x/tmpdir.html).

In this particular example, in the setUp you have:

    def setUp(self) -> None:
        if os.path.exists(REPO_NAME):
            shutil.rmtree(REPO_NAME, onerror=set_write_permission_and_retry)

which means it tries to work with a directory which might exist. While working with a temporary folder, we should always work with one which is just created and unique to this test, and both the pytest fixtures and tempdir module give you that functionality.

I'm happy if you want to keep investigating this, but it's not necessarily a good first issue :)

[artemisep]

Hi @adrinjalali , got it and thanks :)