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

Add PYTEST_TIMEOUT for CircleCI test jobs #18251

Merged

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Jul 22, 2022

⚠️ Before merging, we need to run the full tests on CircleCI to see if there are slower tests that will fail and decide what to do with them.

What does this PR do?

Add PYTEST_TIMEOUT: 30 for CircleCI jobs:

environment:
    ...
    PYTEST_TIMEOUT: 30

The main goal is to avoid CircleCI's default 10 minute timeout that cancels the jobs. Also with this PR, we can see clearly which test (s) timeout.

@ydshieh ydshieh force-pushed the set_pytest_timeout_limit_for_circleci_jobs branch from d953683 to 6a37404 Compare July 22, 2022 09:39
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jul 22, 2022

The documentation is not available anymore as the PR was closed or merged.

@sgugger
Copy link
Collaborator

sgugger commented Jul 22, 2022

I like the idea of getting a hard error instead of silently getting new tests that slow down the CI by quite a lot! Now we just ahve to get to all tests passing below that threshold 😅
For the examples, you can authorize 60s before timing out as those are end-to-end small training so take more time.

@ydshieh
Copy link
Collaborator Author

ydshieh commented Jul 22, 2022

Currently set to PYTEST_TIMEOUT: 120. As mentioned on Slack, tests sometimes get much longer to run. For example,

test_modeling_data2vec_audio.py::Data2VecAudioModelTest::test_mask_time_prob_ctc

44.16s call 
37.64s call     
12.46s call     
12.60s call 

and

test_modeling_plbart.py::PLBartBaseIntegrationTest::test_base_generate

48.18  call
11.20s call 
11.53s call

This makes it difficult to determine a good threshold that won't be flaky.

Current longest 2 tests

(observed on a CircleCI workflow run):

73.08s call     
test_pipelines_image_segmentation.py::ImageSegmentationPipelineTests::test_pt_DetrConfig_DetrForSegmentation_notokenizer_DetrFeatureExtractor

65.33s call
longt5/test_modeling_flax_longt5.py::FlaxLongT5ModelTest::test_jit_compilation

@ydshieh ydshieh force-pushed the set_pytest_timeout_limit_for_circleci_jobs branch from b94f35b to b2407bc Compare July 22, 2022 13:09
setup.py Outdated
@@ -139,7 +139,8 @@
"pytest",
"pytest-timeout",
"pytest-xdist",
"python>=3.7.0",
# To run the full tests -> This will be reverted.
"python>=3.6.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will revert this line once this PR being approved

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

LGTM!

@ydshieh
Copy link
Collaborator Author

ydshieh commented Jul 26, 2022

Reverted the change in setup.py (was for running the full tests). The final timeout limit is 2 minutes (to avoid flaky failures).
I will merge this PR today unless @sgugger has a different opinion.

@ydshieh ydshieh merged commit 6649133 into huggingface:main Jul 26, 2022
@ydshieh ydshieh deleted the set_pytest_timeout_limit_for_circleci_jobs branch July 26, 2022 15:58
oneraghavan pushed a commit to oneraghavan/transformers that referenced this pull request Sep 26, 2022
Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
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.

None yet

4 participants