-
-
Notifications
You must be signed in to change notification settings - Fork 25.5k
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
[MRG] MAINT pytest test discovery #9839
Conversation
Should we use pytest . in CI just to be sure?
…On 27 September 2017 at 18:58, Roman Yurchak ***@***.***> wrote:
Makes sure that py.test . works work scikit-learn, fixes #9699
<#9699> .
------------------------------
You can view, comment on, or merge this pull request online at:
#9839
Commit Summary
- MAINT Ignore benchmarks/ in pytest
- MAINT Ignore doc/ and examples/ in pytest
- MAINT Add setup.py to pytest ignore
File Changes
- *M* setup.cfg
<https://github.com/scikit-learn/scikit-learn/pull/9839/files#diff-0>
(4)
Patch Links:
- https://github.com/scikit-learn/scikit-learn/pull/9839.patch
- https://github.com/scikit-learn/scikit-learn/pull/9839.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#9839>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/AAEz62aU0S5daG3dsbpYzlfwDIdUaJJ6ks5smg5AgaJpZM4Plb2z>
.
|
My initial assumption was that
I don't know how this would fit in. For instance, the doc is currently only tested in Travis CI, scikit-learn/build_tools/travis/test_script.sh Lines 49 to 54 in ea41a78
I don't know with pytest migration you intend to eventually replace the testing via make with pytest (cf #9840)? For instance, with the changes in this PR it could be possible to just run,
Regarding the testing of What do you think? cc @lesteve |
The problem with |
Maybe norecursedirs is what we need? For example see sphinx-gallery setup.cfg |
In the end, I'm not so keen on norecursedirs, and I went with similified
I'm not sure, this would also increase the run time somewhat. I could also affect the coverage btw (in a good way). Overall I think this is an improvement, but note that currently this wouldn't work in CI anyway, because e.g. Travis CI uses pytest's |
I agree it would be convenient to be able to just do The current problems I spotted:
|
Thanks for pointing that out! I agree that writing a |
Merged a more coarse-grained config from #12011. |
If someone want to have |
Makes sure that
py.test .
works work scikit-learn, fixes #9699 .