Skip to content
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

Do we need to test notebooks in every commit of a new PR ? #217

Closed
ismael-mendoza opened this issue Jul 29, 2021 · 4 comments
Closed

Do we need to test notebooks in every commit of a new PR ? #217

ismael-mendoza opened this issue Jul 29, 2021 · 4 comments
Assignees
Labels
tests Everything related to unit tests

Comments

@ismael-mendoza
Copy link
Collaborator

It occurred to me that we might not necessarily want to test notebooks every time we make a new commit in a PR (otherwise we might as well have it as a unit test). Since notebooks are important to maintain but it is not absolutely critical to check that they are not broken, I suggest to only use nbval when there is a push to the (soon to be made) dev branch rather than in every commit of a new PR.

@ismael-mendoza ismael-mendoza added the tests Everything related to unit tests label Jul 29, 2021
@ismael-mendoza ismael-mendoza changed the title Do we need to test notebooks in every PR ? Do we need to test notebooks in every commit of a new PR ? Jul 29, 2021
@thuiop
Copy link
Collaborator

thuiop commented Jan 21, 2022

I'm bumping this issue ; actually, do we need to run the tests at every commit too ? As long as it remains mandatory that they pass before we merge, it should not be necessary to run them every time.

@ismael-mendoza ismael-mendoza self-assigned this Feb 4, 2022
@ismael-mendoza ismael-mendoza pinned this issue Feb 4, 2022
@aboucaud
Copy link
Collaborator

I agree that running tests for every commit pushed into a PR is overkill, especially when the installation is heavy.

But I think for external collaborators, the CI is hold until a maintainer asks for it (when we want to merge basically), which already helps a lot.

Concerning the notebooks, I guess we could make it a separate CI action that runs on a the main branch with a cron every week or so. That would be sufficient IMO.

@ismael-mendoza
Copy link
Collaborator Author

Yes that is exatly what I was thinking :) I'll work on that

@ismael-mendoza
Copy link
Collaborator Author

Closed by #263

@ismael-mendoza ismael-mendoza unpinned this issue Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Everything related to unit tests
Projects
None yet
Development

No branches or pull requests

3 participants