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

[OPT] enable strict_xfail in pytest or remove memory leak tests #6475

Open
gforsyth opened this issue Mar 21, 2025 · 1 comment
Open

[OPT] enable strict_xfail in pytest or remove memory leak tests #6475

gforsyth opened this issue Mar 21, 2025 · 1 comment
Labels
ci Tech Debt Issues related to debt tests Unit testing for project

Comments

@gforsyth
Copy link
Contributor

Every Python run in conda-python-tests-singlegpu runs a set of "memory leak pytests" tests.
These take anywhere from 16 to 18 minutes to complete, and there is currently no useful signal derived from these tests.

One example from earlier today:

https://github.com/rapidsai/cuml/actions/runs/13986886321/job/39162102022#step:9:2140

RAPIDS logger » [03/21/25 08:43:13]
┌───────────────────────────┐
|    memory leak pytests    |
└───────────────────────────┘

/opt/conda/envs/test/lib/python3.12/site-packages/pytest_benchmark/logger.py:39: PytestBenchmarkWarning: Benchmarks are automatically disabled because xdist plugin is active.Benchmarks cannot be performed reliably in a parallelized environment.
  warner(PytestBenchmarkWarning(text))
============================= test session starts ==============================
platform linux -- Python 3.12.9, pytest-7.4.4, pluggy-1.5.0
benchmark: 5.1.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /__w/cuml/cuml/python/cuml
configfile: pyproject.toml
plugins: xdist-3.6.1, hypothesis-6.129.3, cases-3.8.6, benchmark-5.1.0, cov-6.0.0
created: 1/1 worker
1 worker [676 items]

xxxxxxssssssssssssssssssssssssssssssssssssssssssssssssXXXXXXXXXXXXXXXXXX [ 10%]
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX [ 21%]
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX [ 31%]
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXsssssssssssssssssssssssssssssssss [ 42%]
ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 53%]
ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 63%]
ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 74%]
ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 85%]
ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss [ 95%]
sssssssssxssssssssxssssssssx                                             [100%]
---- generated xml file: /__w/cuml/cuml/test-results/junit-cuml-memleak.xml ----

---------- coverage: platform linux, python 3.12.9-final-0 -----------
Coverage XML written to file /__w/cuml/cuml/coverage-results/cuml-memleak-coverage.xml

=========== 466 skipped, 9 xfailed, 201 xpassed in 966.75s (0:16:06) ===========

Every test in there is either skipped, xfailed, or is actually XPASSing.

If half the XPASSing tests started xfailing again, there would be no indication of that regression unless someone noticed the lower-case x marks.

I don't think this (quite long) test bundle is accomplishing anything and I would recommend either:

  1. Ripping it all out and saving 18 minutes per-worker per-run
  2. Enabling strict xfail in pytest so that XPASSing tests are marked as failures and then remove the defunct xfail markers.
  3. Start from scratch with a well-curated set of ideas / failure modes that we want to test.
@gforsyth gforsyth added ? - Needs Triage Need team to review and classify bug Something isn't working labels Mar 21, 2025
@jcrist jcrist added tests Unit testing for project ci Tech Debt Issues related to debt and removed bug Something isn't working ? - Needs Triage Need team to review and classify labels Mar 21, 2025
@jcrist
Copy link
Member

jcrist commented Mar 21, 2025

Thanks for opening this Gil! I agree that this test run appears not to be adding any real value here. I'm not sure the history behind the memleak tests and whether scrapping or improving them would make more sense. cc @dantegd for historical context here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Tech Debt Issues related to debt tests Unit testing for project
Projects
None yet
Development

No branches or pull requests

2 participants