narwhals-dev / narwhals

Lightweight and extensible compatibility layer between dataframe libraries!
https://narwhals-dev.github.io/narwhals/
MIT License
423 stars 76 forks source link

test: use pytest-randomly, recommend pytest-xdist in docs #1019

Closed TNieuwdorp closed 1 week ago

TNieuwdorp commented 1 week ago

What type of PR is this? (check all applicable)

Related issues

Checklist

If you have comments or can explain your changes, please do so below.

After discussion with Francesco he deemed it okay to implement the multithreaded variant to speed up the 3800 unit tests on any system that has more than 1 core. Although pytest is not designed with multithreading in mind, all tests complete succesfully.

MarcoGorelli commented 1 week ago

thanks @TNieuwdorp for your pr!

i'm slightly hesistant about enabling this by default, as running pytest multiprocessed means you can't run it with --pdb (something I do very often...you can do -n 0, but still, it's an extra command to remember when debugging)

maybe we could just enable this one (and pytest-randomly while we're here) but only in CI?

@FBruzzesi any thoughts on this?

FBruzzesi commented 1 week ago

thanks @TNieuwdorp for your pr!

i'm slightly hesistant about enabling this by default, as running pytest multiprocessed means you can't run it with --pdb (something I do very often...you can do -n 0, but still, it's an extra command to remember when debugging)

maybe we could just enable this one (and pytest-randomly while we're here) but only in CI?

@FBruzzesi any thoughts on this?

I don't think in CI makes much of a difference as runners have 2 cores and there is some overhead to account for - I am double checking by comparing the numbers with other PRs pytest runtime.

Maybe we can mention in the docs/contributing guide how to speed up testing locally and that's all for now?