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

Make PR code-tests more user friendly #934

Open
gerdm opened this issue Jun 26, 2022 · 2 comments
Open

Make PR code-tests more user friendly #934

gerdm opened this issue Jun 26, 2022 · 2 comments

Comments

@gerdm
Copy link
Collaborator

gerdm commented Jun 26, 2022

Making a PR with appropriate code seems to be independent of the result for the code checks. This makes it hard to test if the PR checks are failing because of my code or something else.

For instance, in #933, I created a new notebook and did not modify any other notebook. There are 24 tests, I passed 4 and failed 20. Looking through some of the unit tests that failed, I see that 183 notebooks were tested and one failed. I got the error:

Oh no! 💥 💔 💥
1 file would be reformatted, 183 files would be left unchanged.
Error: Process completed with exit code 1.

But I didn't reformat any existing notebook


All of the failures seem to be completely outside the scope of my PR.

=================================== FAILURES ===================================
[107](https://github.com/probml/pyprobml/runs/7059536084?check_suite_focus=true#step:7:108)
__________ test_run_notebooks[notebooks/book2/29/ggm_fit_demo.ipynb] ___________
[108](https://github.com/probml/pyprobml/runs/7059536084?check_suite_focus=true#step:7:109)
[gw1] linux -- Python 3.7.13 /github/home/miniconda3/envs/py37/bin/python
[109](https://github.com/probml/pyprobml/runs/7059536084?check_suite_focus=true#step:7:110)

Suggestions:

  1. Unit-test only the modified / changed notebook
  2. Create an .md file describing what each unit-test is doing so that people who make PRs understand what fails.
@patel-zeel
Copy link
Collaborator

@gerdm Yes, you are right about confusing workflow for the contributors. The first check checks if notebooks are black formatted or not. It seems that your notebook needs black formatting. Also, have you installed pre-commit locally? because ideally, it should have prevented the code_quality check from failing.

@patel-zeel
Copy link
Collaborator

@gerdm I have made some quick changes in name of the jobs to make it a bit easier. I have also implemented your suggestion of testing only the modified/changed notebooks. Can you pull the master in #933 and see if it works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants