Skip to content

CircleCI default behavior running html-noplot when no example is modified #10943

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

Closed
albertcthomas opened this issue Apr 9, 2018 · 6 comments
Closed

Comments

@albertcthomas
Copy link
Contributor

Isn't it bad that CircleCI only runs html-noplot when no example is modified? Changing the code of an estimator can break some examples but CircleCI won't fail with html-noplot.

@qinhanmin2014
Copy link
Member

Questions here: Any example for situations when tests pass but examples fails (except for HTTP errors)? I can't recall such situation in the past year. Maybe we should rely on the tests to ensure the correctness of the code?
Another concern is time, 1 complete job takes over 40 minutes.

@albertcthomas
Copy link
Contributor Author

In PR #10700, CircleCI tests are OK but I think examples/covariance/plot_outlier_detection.py should have failed because I removed the _decision_function method from LOF and _decision_function is used in examples/covariance/plot_outlier_detection.py

@qinhanmin2014
Copy link
Member

@albertcthomas Thanks for the example. I suppose we should blame the example of using _decision_function?
I vote +0 here since Circle will needs more than 40min instead of around 10min now.

@albertcthomas
Copy link
Contributor Author

albertcthomas commented Apr 9, 2018

PR #10700 might not be a good example after all because some examples have been modified and in such a case I think Circle only tests the modified examples and not the other ones (including plot_outlier_detection.py). But is this the intended behavior? Event if this would mean that we could merge a PR breaking an example whereas the full build of the doc would have revealed the problem.

@lesteve
Copy link
Member

lesteve commented Apr 10, 2018

This was done like this in order for PR CIs to finish quicker as @qinhanmin2014 was saying. This is not perfect but covers very well the 95% of cases.

This is not really a problem because master always does a full build, so in the rare case when a change to an estimator does break an example, master fails and we ask nicely the original author of the PR to fix it.

FYI, you can add [doc build] in your commit message if you want the doc to build.

I am going to close this one, feel free to reopen if you disagree or can think of a better approach.

@lesteve lesteve closed this as completed Apr 10, 2018
@albertcthomas
Copy link
Contributor Author

thanks @lesteve. This answered my question and helped me understand the intended behavior of Circle which can be useful when reviewing or working on PRs :). Thanks again @qinhanmin2014.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants