Skip to content

WIP: Add unified test suite infrastructure#353

Closed
lesteve wants to merge 3 commits into
dask:mainfrom
lesteve:common-tests
Closed

WIP: Add unified test suite infrastructure#353
lesteve wants to merge 3 commits into
dask:mainfrom
lesteve:common-tests

Conversation

@lesteve

@lesteve lesteve commented Oct 15, 2019

Copy link
Copy Markdown
Member

Closes #8.

The idea is that there is a lot of tests that we would want to run for all the FooCluster classes. At the moment there is some duplication in test_sge.py, test_slurm.py, etc ... (e.g. test_basic, test_adaptive, ...) and some cluster-generic functionality are only tested for some clusters (e.g. only test_pbs.py has some tests for scaling with "grouped workers" e.g. test_scale_grouped).

This is quite WIP at the moment but if you have some early feed-back about the approach don't hesitate! The main thing to look at is test/test_cluster.py. The way it currently works is you add a check_whatever(cluster, client) function in tests/test_cluster.py and there is a parametrized test function at the end that will run the tests for all the check_* functions and all the FooCluster classes.

This is somewhat influenced by how the "common tests" are structured in scikit-learn (this tests some common properties of all the scikit-learn estimators, for example that fitting twice with the same data gives the same result) and there may be a more clever way of doing it.

A few things I want to look at still (besides cleaning up all the TODO):

  • it would be great to differentiate check functions that need a real cluster (basically that need to do .scale). This could be based on a naming convention e.g. check_real_cluster vs check or be in different files test_real_cluster.py vs test_cluster.py (but then there would be some duplication). Maybe a clever pytest trick would do it (can we add pytest marks to normal functions and detect that) or even use a simple decorator that adds an attributed to the function and we can test this attribute in the parmetrized test.
  • some documentation in the module and/or in a contributing guide
  • potentially other things that I will add later

@mrocklin

Copy link
Copy Markdown
Member

In principle this seems fine to me. Thank you for paying attention to the test suite. I agree that it could be better unified.

@lesteve lesteve added this to the 0.8 milestone Dec 1, 2019
@guillaumeeb

Copy link
Copy Markdown
Member

@lesteve let us know when this is ready!

@lesteve lesteve force-pushed the master branch 2 times, most recently from 4d181fe to 26a0e70 Compare December 8, 2020 12:40
@andersy005 andersy005 added the CI Continuous Integration tools label Jan 24, 2021
Base automatically changed from master to main February 10, 2021 07:12
@guillaumeeb guillaumeeb removed this from the 0.8 milestone Aug 30, 2022
@lesteve

lesteve commented Oct 18, 2022

Copy link
Copy Markdown
Member Author

Unlikely to work on this anytime soon, closing.

@lesteve lesteve closed this Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous Integration tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unified test suite

4 participants