8000 [MRG] Refactors doc test into seperate script by thomasjpfan · Pull Request #12248 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG] Refactors doc test into seperate script #12248

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 6, 2018
Merged

[MRG] Refactors doc test into seperate script #12248

merged 2 commits into from
Oct 6, 2018

Conversation

thomasjpfan
Copy link
Member

Reference Issues/PRs

Fixes #12237

What does this implement/fix? Explain your changes.

This fix runs the doc tests independently of the module tests. When either one fails, the CI will fail.

@thomasjpfan thomasjpfan changed the title TST: Refactors doc test into seperate script MRG: Refactors doc test into seperate script Oct 2, 2018
@thomasjpfan thomasjpfan changed the title MRG: Refactors doc test into seperate script [MRG] Refactors doc test into seperate script Oct 2, 2018
@thomasjpfan thomasjpfan changed the title [MRG] Refactors doc test into seperate script [WIP] Refactors doc test into seperate script Oct 2, 2018
@qinhanmin2014
Copy link
Member

Closing, see the issue, thanks for your PR.

@qinhanmin2014 qinhanmin2014 reopened this Oct 3, 2018
NicolasHug
NicolasHug reviewed Oct 3, 2018
View reviewed changes
build_tools/travis/test_pytest_soft_dependency.sh

set -e

if [[ "$CHECK_PYTEST_SOFT_DEPENDENCY" == "true" ]]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity why did you move this out of test_script.sh?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original test_script.sh order was:

  1. Run pytest scikit-learn modules.
  2. Run doc-test.
  3. Run check_pytest_soft_dependency, which removes pytest.

If check_pytest_soft_dependency was kept in test_script.sh, pytest would be removed before doc-test gets to run.

jnothman
jnothman reviewed Oct 4, 2018
View reviewed changes
Copy link
Member
@jnothman jnothman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this WIP? The only problem I can see is that now test_docs and test_pytest_soft_dependency are also being run on the flake8 check. Maybe the flake8 check should be done by overriding script rather than by setting an env var.

@NicolasHug
Copy link
Member

The only problem I can see is that now test_docs and test_pytest_soft_dependency are also being run on the flake8 check

But from travis.yml it looks like the flake8 test sets SKIP_TESTS="true" and doesn't set CHECK_PYTEST_SOFT_DEPENDENCY, so surely test_docs and test_pytest_soft_dependency will be ignored?

@thomasjpfan thomasjpfan changed the title [WIP] Refactors doc test into seperate script [MRG] Refactors doc test into seperate script Oct 4, 2018
@thomasjpfan
Copy link
Member Author

From the latest Travis run:
https://travis-ci.org/scikit-learn/scikit-learn/builds/436629799 when flake8 runs, test_docs and test_pytest_soft_dependency does not run.

@jnothman jnothman merged commit 3804ccd into scikit-learn:master Oct 6, 2018
jnothman pushed a commit to jnothman/scikit-learn that referenced this pull request Oct 15, 2018
@thomasjpfan @jnothman
MNT Refactors doc test into seperate script (scikit-learn#12248)
42049b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnothman jnothman jnothman approved these changes

@NicolasHug NicolasHug NicolasHug approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Travis does not run make test-doc when pytest fails
4 participants
@thomasjpfan @qinhanmin2014 @NicolasHug @jnothman
0