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

[FEATURE] remove notebook unit-tests #2043

Closed
loomlike opened this issue Dec 18, 2023 · 2 comments
Closed

[FEATURE] remove notebook unit-tests #2043

loomlike opened this issue Dec 18, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@loomlike
Copy link
Collaborator

Description

We have notebook tests for unit/, smoke/, and functional/
but the unit and smoke notebook tests are covering about the same stuffs, basic tests of the notebook. Also I found many notebooks already are not in unit tests.

I think we can remove all the notebook tests from unit and just keep smoke and functional notebook tests. Any thoughts? @miguelgfierro @SimonYansenZhao @anargyri

@loomlike loomlike added the enhancement New feature or request label Dec 18, 2023
@miguelgfierro
Copy link
Collaborator

The unit tests in notebooks cover the basic work of the notebook. It is actually an innovation we introduced, in a similar way that unit tests in functions make sure that the function works properly, the unit tests in notebooks make sure that they work.
In the case of functional tests, we are not only checking that the notebooks work, but also that they provide the right values.

For example, a notebook that outputs an RMSE of -1 will pass the unit test because the notebook works, but the functional notebook will break because an RMSE of -1 is incorrect.

The smoke test is just a guardrail for functional or integration tests that take too long, and they go sequentially with the functional test. If a functional test takes 1h runing for 10 epochs, we have a smoke test that is the same notebook but maybe runs for 1 epoch. They are used to check for quick errors.

Consider that the test pipeline we have is not only used for making sure that the repo is healthy but to showcase a pattern in machine learning projects.

We reviewed the categories even with Eric Gamma, who was the creator of junit.

@miguelgfierro
Copy link
Collaborator

Closing this, feel free to open to open if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants