Add: code coverage & CI pages to guide#104
Conversation
There was a problem hiding this comment.
thanks for your work on this leah :):):):) this is SUPER IMPORTANT and i have been needing something like this for so long.
I think you cover the important points well here, my comments are mostly nitpicky things that could be improved, but take or leave all of them!
One general comment i would have is that tests are also related to the structure of code, so a short section on "writing testable code" and how tests can force you to write better code just by virtue of how messy code is really difficult to write tests for might be useful? - so an example would be a bigass MATLAB style scientific analysis function that is like 1000 lines long and it loads/saves a bunch of files, plots inline, etc. I have a (super brief, only had a few hours to put together this thing) example here
I also think that a very minimal working example repo would be lovely to have here - eg. if the section could take the example you have with the temperature sensor and keep it running throughout - we write a few functions from different parts of the "program" and then write the corresponding tests so we can have examples for everything, and then when we get to the actually running your tests part we can just literally run the tests for our code, demonstrate what it looks like when a test fails, how to fix it, etc. with screenshots of pytest, nox, what it looks like in a pull request when you have CI enabled, etc. (I'd be happy to help write those and also any of the other recommendations i make here!!!! not trying to just demand labor from you and not volunteer!!!!)
| Test types: unit, integration, functional <test-types> | ||
| Run tests in CI & locally <run-tests> | ||
| Package data <data> | ||
| CI <ci> |
There was a problem hiding this comment.
Should this be a separate page, or in run-tests?
There was a problem hiding this comment.
i can't decide actually and would love feedback. this is why i kind of kept that landing page blank.
I feel like for beginners we want to have a CI page somewhere that introduces it as a concept and what it can do for them.
but then a user may want to set it up for both doc builds, tests and the actual package build. so it could go into 3 different parts of this guide. so MAYBE we just have the CI for tests in this section? and the general CI page lives... somewhere else in the guide? i'm not sure and welcome feedback.
|
|
||
| Writing sets of test functions and methods, also known as test suites, for your | ||
| package is important for both you as a maintainer, your users, and any potential | ||
| new contributors. Test suites consist of sets of functions, methods, and classes |
There was a problem hiding this comment.
sort of redundant. maybe "Each function, method, and class makes sure a ..."
|
|
||
| ## Why create tests for your package | ||
|
|
||
| Tests act as a safety net for code changes. They help you spot and rectify bugs |
There was a problem hiding this comment.
Bullet points?
- Avoid breaking things - tests make sure that new features or bugfixes don't break things in the package. If you don't have a formal test suite, you probably already run tests, just manually! Tests automate all the manual checks you might do when making changes to your code and allow you to make changes with confidence
- Collaboration - tests make it possible for people outside your team to contribute to the project without needing an exhaustive code review, and without them needing to understand every detail of the project. If you have tests, and the tests pass with the new code, you've avoided all the labor that usually goes into reviewing and contributing.
- Edge cases - ...
| @@ -0,0 +1,16 @@ | |||
| # What is continuous integration? | |||
There was a problem hiding this comment.
Where this page lives in the guide is still not known... it will be fleshed out to contain an overview of what CI is, why it's useful and various platform options focusing on github actions examples that people can use out of the box.
| run: | | ||
| echo "PY=$(python -c 'import hashlib, sys;print(hashlib.sha256(sys.version.encode()+sys.executable.encode()).hexdigest())')" >> $GITHUB_OUTPUT | ||
| echo "PIP_CACHE=$(pip cache dir)" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Being able to cache things (beyond the cache: pip thing I mentioned above) is a great thing to include! I have a feeling a lot of people will want to customize caching in some way. Would it help to add a comment describing what these lines do?
There was a problem hiding this comment.
@namurphy it would be super helpful. are you able to suggest some language? i absolutely understand caching and the value of caching but i don't understand why
- above we use pip as the cache value - why?
- what does this section generally do?
many thanks!! caching is a great addition to this and we can mention it too in our main Ci pages / tutorials.
Zeitsperre
left a comment
Zeitsperre
left a comment
There was a problem hiding this comment.
Just a quick overview, looking great!
|
@willingc this is the PR that i'd love to tackle next week. it's been open since september. i will clean up all of the conflicts and prioritize it for tuesday when i'm back online! i'd love to have your eyes on it as well if you can help review! ✨ |
|
ok y'all. i am going to focus on cleaning up this PR for the next week. a few things you'll see me doing
|
lwasser
force-pushed
the
tests
branch
3 times, most recently
from
b875356 to
8fc5e1e
Compare
ENH: fixes from Jonny's review Fix: review edits from Jonny p2 Fix: typos and cleanup Fix: add example to tests ci page
Co-authored-by: Nick Murphy <[email protected]> Update ci-tests-data/ci.md Co-authored-by: Nick Murphy <[email protected]> Update ci-tests-data/ci.md Co-authored-by: Nick Murphy <[email protected]> Update ci-tests-data/ci.md Co-authored-by: Nick Murphy <[email protected]> Update ci-tests-data/ci.md Co-authored-by: Nick Murphy <[email protected]> Update ci-tests-data/ci.md Co-authored-by: Nick Murphy <[email protected]> Update ci-tests-data/code-cov.md Co-authored-by: Nick Murphy <[email protected]> Fix: edits from @namurphy to run tests page Co-authored-by: Nick Murphy <[email protected]> Update ci-tests-data/run-tests.md Co-authored-by: Nick Murphy <[email protected]>
Co-authored-by: Trevor James Smith <[email protected]> Fix: edit from trevor Co-authored-by: Trevor James Smith <[email protected]> Fix: edit from trevor Co-authored-by: Trevor James Smith <[email protected]> Fix: edits from Trevor Co-authored-by: Trevor James Smith <[email protected]> Fix: edits from Nick Co-authored-by: Nick Murphy <[email protected]> Packaging image Fix: remove unused block Fix Update ci-tests-data/tests-ci.md Co-authored-by: Trevor James Smith <[email protected]>
Co-authored-by: Carol Willing <[email protected]> Update ci-tests-data/ci.md Co-authored-by: Carol Willing <[email protected]> Update ci-tests-data/ci.md Co-authored-by: Carol Willing <[email protected]> Fix: other edits from review'
lwasser
changed the title
5 participants
This is a draft for community developed text that i am now turning into a PR for the tests section.