Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move some test files (tets/test_xxx_utils.py) to tests/utils #31730

Merged
merged 5 commits into from
Jul 2, 2024

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Jul 1, 2024

What does this PR do?

For our daily CI (and other scheduled CI) workflow, we collect the tests by using the folders under tests. However, there are some files under tests too. For some test files, they contains TestCase but their tests are not collected in the CI workflow run.

This PR moves tets/test_xxx_utils.py into tests/utils.

To be very sure we don't miss any test, it's probably better to come up with a solution that will collect the tests from the files under tests but not in any of its sub-directory. But it's more complex and might take a bit more time to deal with.

@ydshieh ydshieh marked this pull request as draft July 1, 2024 13:37
@ydshieh
Copy link
Collaborator Author

ydshieh commented Jul 1, 2024

Hi @amyeroberts. The failures are irrelevant I think

  • ERROR tests/utils/test_file_utils.py - RuntimeError: Failed to import transformers.models.gpt2.tokenization_gpt2_tf because of the following error (look up to see its traceback): cannot import name 'ops' from 'keras' (/usr/local/lib/python3.10/site-packages/keras/__init__.py)'

  • FAILED tests/utils/test_chat_template_utils.py::JsonSchemaGeneratorTest::test_multiple_complex_arguments - AssertionError: {'nam[195 chars]': ['string', 'integer'], 'nullable': True, 'd[47 chars]x']}} != {'nam[195 chars]': ['integer', 'string'], 'nullable': True, 'd[47 chars]x']}}

But maybe it's better to wait them being fixed ?

@ydshieh ydshieh changed the title [WIP] Move some test files Move some test files (tets/test_xxx_utils.py) to tests/utils Jul 1, 2024
@ydshieh ydshieh requested review from amyeroberts and SunMarc July 1, 2024 14:32
@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.

@SunMarc
Copy link
Member

SunMarc commented Jul 1, 2024

Yeah it's irrelevant. Let's wait until the CI is fixed !

Copy link
Member

@SunMarc SunMarc left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @ydshieh !

Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

tests/utils/test_image_processing_utils.py Outdated Show resolved Hide resolved
@ydshieh ydshieh force-pushed the move_test_files branch from 95d7584 to 78f104d Compare July 2, 2024 08:49
@ydshieh ydshieh requested a review from amyeroberts July 2, 2024 09:15
@ydshieh ydshieh marked this pull request as ready for review July 2, 2024 09:15
Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks!

@ydshieh ydshieh merged commit 93cd94b into main Jul 2, 2024
20 of 22 checks passed
@ydshieh ydshieh deleted the move_test_files branch July 2, 2024 11:46
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.

4 participants