TST: Added tests checking entry_points discovery and config initialisation #89
Conversation
| @@ -4,6 +4,7 @@ | |||
| import nx_parallel as nxp | |||
|
|
|||
|
|
|||
| @pytest.mark.order(2) | |||
There was a problem hiding this comment.
I suspect this isn't directly related to the point of this PR, but since we're here:
At the bottom of this test, it looks like we are checking that n_jobs=0 raises a specific ValueError.
Can those 5 lines be done using pytest.raises? Something like:
with pytest.raises(ValueError, match="n_jobs == 0 in Parallel has no meaning"):
nxp.get_n_jobs(0)There was a problem hiding this comment.
yes, and I think we can do that in a different PR
| ) | ||
|
|
||
|
|
||
| @pytest.mark.order(1) |
There was a problem hiding this comment.
Why does the order matter here?
- If the
order(2)test is run first, how does that affect this one? - Is it that we need side effects from this test in the setup of the other test?
There was a problem hiding this comment.
because configurations are global, so the ordering of the tests matter
There was a problem hiding this comment.
I now understand why the order(1) needs to be first -- otherwise the nx.config values might have been changed by the other tests (there are side effects of the tests).
And if I understand corretly the order(2) needs to be run before the others so we can test that the default number of n_jobs is 1.
Should we be managing the nx.configs around all the tests?
Maybe with an automated setup for each test that resets nx.config to its default values?
That might be simpler code-wise than relying on test order -- but it also might slow down the tests.
Anyway, I approve this PR as is and we can switch to a universal setup of the nx.config in another PR if we want.
Thanks!!
and updated networkx version to
3.4.2