Skip to content

TST: Refactor unittest to pytest style custom tests#2573

Merged
BenjaminBossan merged 1 commit intohuggingface:mainfrom
BenjaminBossan:tst-refactor-custom-tests-pytest
Jun 6, 2025
Merged

TST: Refactor unittest to pytest style custom tests#2573
BenjaminBossan merged 1 commit intohuggingface:mainfrom
BenjaminBossan:tst-refactor-custom-tests-pytest

Conversation

@BenjaminBossan
Copy link
Member

This is a follow up to #2462, #2478, and #2491.

Description

Finish the refactor from unittest-style tests to pytest-style tests to now also include the last big file to still use the old style, test_custom_models.py. This file was already mostly written with pytest in mind, so the changes were rather minimal.

With this class refactored, we can finally remove ClassInstantier, which made understanding test parametrization much more difficult.

There are still a few unittest.TestCase classes found throughout, but they are pretty isolated, so no big need to refactor them right now.

Test coverage

Note that the test coverage is the same except for one difference. For some reason, I found that module level __getattr__ is no longer called when moving from unittest to pytest, such as here:

def __getattr__(name):

This was pretty strange. I found out that these lines are invoked by usage of self.assertWarnsRegex, which is strange, as it appears unrelated. The reason seems to be the unittest code here:

https://github.com/python/cpython/blob/9258f3da9175134d03f2c8c7c7eed223802ad945/Lib/unittest/case.py#L296-L305

Therefore, I think it's safe to ignore this reduction in test coverage.

This is a follow up to huggingface#2462, huggingface#2478, and huggingface#2491.

Finish the refactor from unittest-style tests to pytest-style tests to
now also include the last big file to still use the old style,
test_custom_models.py. This file was already mostly written with pytest
in mind, so the changes were rather minimal.

With this class refactored, we can finally remove ClassInstantier, which
made understanding test parametrization much more difficult.

There are still a few unittest.TestCase classes found throughout, but
they are pretty isolated, so no big need to refactor them right now.

Note that the test coverage is the same except for one difference. For
some reason, I found that module level __getattr__ is no longer called
when moving from unittest to pytest, such as here:

https://github.com/huggingface/peft/blob/62c9cf30319ca219e1d6754626c17f510fc76441/src/peft/tuners/adalora/__init__.py#L32

This was pretty strange. I found out that these lines are invoked by
usage of self.assertWarnsRegex, which is strange. The reason seems to be
the unittest code here:

https://github.com/python/cpython/blob/9258f3da9175134d03f2c8c7c7eed223802ad945/Lib/unittest/case.py#L296-L305

Therefore, I think it's safe to ignore this reduction in test coverage.
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@BenjaminBossan BenjaminBossan requested a review from githubnemo June 5, 2025 14:56
Copy link
Collaborator

@githubnemo githubnemo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@BenjaminBossan BenjaminBossan merged commit f53dd49 into huggingface:main Jun 6, 2025
18 of 27 checks passed
@BenjaminBossan BenjaminBossan deleted the tst-refactor-custom-tests-pytest branch June 6, 2025 10:21
efraimdahl pushed a commit to efraimdahl/peft that referenced this pull request Jul 12, 2025
This is a follow up to huggingface#2462, huggingface#2478, and huggingface#2491.

Finish the refactor from unittest-style tests to pytest-style tests to
now also include the last big file to still use the old style,
test_custom_models.py. This file was already mostly written with pytest
in mind, so the changes were rather minimal.

With this class refactored, we can finally remove ClassInstantier, which
made understanding test parametrization much more difficult.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants