Skip to content

[MNT] Diagnose and address long test runtimes (#1633)#1692

Open
Abhishek9639 wants to merge 3 commits intoopenml:mainfrom
Abhishek9639:mnt/diagnose-long-test-runtimes
Open

[MNT] Diagnose and address long test runtimes (#1633)#1692
Abhishek9639 wants to merge 3 commits intoopenml:mainfrom
Abhishek9639:mnt/diagnose-long-test-runtimes

Conversation

@Abhishek9639
Copy link
Copy Markdown

@Abhishek9639 Abhishek9639 commented Feb 26, 2026

[MNT] diagnose and address long test runtimes. Closes #1633

Changes

Current CI test runs take 1–2+ hours. This PR diagnoses the bottleneck and implements several improvements:

Root Cause

The production_server tests (74 tests) make live API calls to openml.org, taking ~1h 23m in CI even with 4-worker parallelization.

Improvements

  1. Global per-test timeout (pyproject.toml)

    • Added timeout = 600 (10 min) to [tool.pytest.ini_options]
    • Prevents any single test from hanging indefinitely
  2. CI workflow improvements (.github/workflows/test.yml)

    • Changed --durations=20--durations=0 to report ALL test durations for diagnosis
    • Added explicit --timeout=600 to all 3 pytest invocations
  3. Fixture optimization (tests/conftest.py)

    • Changed verify_cache_state fixture scope from functionmodule
    • Reduces redundant filesystem I/O (was running before/after EVERY test)
  4. Benchmark script (scripts/profile_tests.sh)

    • New script for easy local test duration profiling
    • Configurable marker filters

Test Distribution Analysis

Category Count CI Time
All tests 368
production_server 74 ~1h 23m (bottleneck)
test_server 196 excluded from CI
sklearn-only 6 ~1 min
Non-server 99 fast

Verification

  • All pre-commit checks pass (ruff, ruff-format, mypy)
  • All 368 tests still collect correctly

@Abhishek9639
Copy link
Copy Markdown
Author

Abhishek9639 commented Feb 26, 2026

Hii @geetu040 and @fkiraly,
Fixed the code quality checks. All pre-commit checks are now passing.
Please review it.

Copy link
Copy Markdown
Collaborator

@geetu040 geetu040 left a comment

Choose a reason for hiding this comment

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

Thanks, scripts/profile_tests.sh file will be used, but I not sure about other duration and timeout related changes. See comments below.

@geetu040
Copy link
Copy Markdown
Collaborator

geetu040 commented Mar 1, 2026

you should mention the issue #1633 without the keyword Fixes #1633 since it doesn't close it, rather adds script to help debug this.

@Abhishek9639
Copy link
Copy Markdown
Author

@geetu040,
I’ve made all the changes you suggested. Could you please review it once?
And if any further changes are needed, please let me know.
Thanks

Copy link
Copy Markdown
Collaborator

@geetu040 geetu040 left a comment

Choose a reason for hiding this comment

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

see the comment below

@Abhishek9639
Copy link
Copy Markdown
Author

@geetu040,
Updated Added -n for workers with --dist=load and removed -q for full output the script now mimics the exact CI pytest command. Please review
If any further changes are needed, please let me know.
Thanks

Copy link
Copy Markdown
Collaborator

@geetu040 geetu040 left a comment

Choose a reason for hiding this comment

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

LGTM

@fkiraly please merge.

@Abhishek9639 Abhishek9639 reopened this Mar 7, 2026
Abhishek added 3 commits April 7, 2026 17:15
- Add global per-test timeout (600s) to pytest config
- CI: report all test durations (--durations=0) for diagnosis
- CI: add explicit --timeout=600 to prevent hanging tests
- Optimize verify_cache_state fixture: scope function -> module
- Add scripts/profile_tests.sh for local duration profiling
…script

- Revert CI workflow to original --durations=20 (no timeout)
- Remove global timeout from pyproject.toml
- Revert conftest.py verify_cache_state scope to function
- Update profile_tests.sh: accept CLI args (-m, -d, -t, -o) with defaults
- Add -n flag for parallel workers (default: 4)
- Add --dist=load to distribute tests across workers
- Remove -q flag for full pytest output
- Mimics exact pytest command used in CI
@Abhishek9639 Abhishek9639 force-pushed the mnt/diagnose-long-test-runtimes branch from 7114df4 to 144cee9 Compare April 7, 2026 11:46
@PGijsbers
Copy link
Copy Markdown
Collaborator

Is this ready for review or not?:) I was pinged for this but I see there are still commits being pushed

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.67%. Comparing base (e653ef6) to head (144cee9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1692   +/-   ##
=======================================
  Coverage   54.67%   54.67%           
=======================================
  Files          63       63           
  Lines        5108     5108           
=======================================
  Hits         2793     2793           
  Misses       2315     2315           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Abhishek9639
Copy link
Copy Markdown
Author

Hi @PGijsbers,
Yes, it's ready for review now! Sorry about the confusion the force-push was just to clean up the commit history.
No more changes planned.
Thanks

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.

[MNT] diagnose and address long test runtimes

4 participants