Skip to content

Conversation

@ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Nov 14, 2025

What does this PR do?

Reduce timing on CircleCI.

Use @slow for some IntegrationTests.

Note that, for IntegrationTest class that have setupClass, even if a test method is fast, the setupClass might still take quite long time, if it downloads models. Even tokenizers may take 3 -5 seconds sometimes

@github-actions
Copy link
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: aria, bark, hiera, imagegpt, jamba, llava, mamba, mbart, olmo2, plbart, rag, rt_detr, rt_detr_v2, speech_to_text, speecht5, tvp

@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.

Copy link
Member

@Cyrilvallez Cyrilvallez left a comment

Choose a reason for hiding this comment

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

Makes a lot of sense, except the imagegpt one! I think this is unrelated!

Comment on lines +252 to +254
# After #33632, this test still passes, but many subsequential tests fail with `device-side assert triggered`.
# We need to put `@slow` whenever `run_test_using_subprocess` is used.
@slow
Copy link
Member

Choose a reason for hiding this comment

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

I don't see how this can can the case when this is run on the CI? Because it does not have any accelerator available so no device-side assert triggered

Probably the issue is only the @run_test_using_subprocess which should not be necesary here I think!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This comment is from my previous PR #34213, and I am sure I did the investigation at that time :-).

This test call the one in the parent class GenerationTesterMixin in the file tests/generation/test_utils.py and it does use torch_device.

Of course, when running on CircleCI, the torch_device is CPU and the comment doesn't apply in this case. But we still need put run_test_using_subprocess so the problem won't occur when it's running on GPU enviornment.

I can of course make run_test_using_subprocess more flexible to accept some conditions, but it won't be done in this PR .

Let me know your opinion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Merge now. Will address if there is further comment (which should be in a follow-up PR anyway)

@ydshieh ydshieh merged commit cd416f3 into main Nov 18, 2025
18 checks passed
@ydshieh ydshieh deleted the fix_tests_being_slow_001 branch November 18, 2025 09:17
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