Skip to content

Conversation

@Bluesy1
Copy link
Collaborator

@Bluesy1 Bluesy1 commented Jun 16, 2023

Adds pytest-cov as a dev dependency to enable simple coverage generation from pytest.

Coverage is automatically calculated and emitted to the terminal output when running poetry run pytest. Actions Workflow now generates a coverage.xml file, which can be pushed to a coverage reporting provider of choice, if any, and runs on python 3.9 since thats the version specified in the readme file, and python 3.8 is EOL in just over a year. On top of standard line coverage, branch coverage is also enabled to check if all paths of conditionals are considered.

I also updated the tests to properly use the xfail setup for excluding files, to see which tests xfailed/xpassed add -rxX to your pytest invocation (see how tests are run in the GHA workflow for an example)

Since I was adding a new depenency for pytest-cov and black, I updated pyproject.toml to use the new dev dependency format, treating it like an extras group - instead of being under the [tool.poetry.dev-dependencies] table, they now reside under the [tool.poetry.group.dev.dependencies] (where poetry added pytest-cov when I ran poetry add pytest-cov -G dev).

While I know you said that you weren't keen on adding pre-commit to here in #39, its the easiest way to add black here for people, so they don't have to remember to run it locally, or setup their editor to auto format code, which is editor dependent. Its a super simple config, just to run black and some other formatting things. I can remove it if you want, I specifically made sure not to add it as a dependency to the project, so its completely optional.

Black ignores any files in the test problems subdirectories to not cause false test failures, and has a line length limit overridden to 100 instead of its default of 88 (My personal opinion is PEP8's line length limit is too low, so I always increase black's default). I also have it set to require the same major version as I set it up with, as every comes a new set of formatting from it, and its not nice to have black suddenly start failing every January when the first new release of every comes out.

Mostly Resolves #39, though it doesn't add linting to md files here. If wanted, I'll setup a PR to the instructor datascience bank as an example to add prettier to lint markdown files there, with pre-commit and/or gh actions, where formatting markdown files is more important.

Sidenote, due to adding in black, you may want to consider a .git-blame-ignore-revs file with the following content to remove commits from black formatting from screwing up and git blames:

96439ba741d2d78f31ac6ed5c93da1ca32dc539a # initial black on src/* files
3caf569c76273ab77f9982b825c3b9165dd8fe11 # initial black run on docs/* files and tests/test_problem_bank_scripts.py

Bluesy1 added 9 commits June 15, 2023 07:29
In `tests.yml`, there is also now a commented job step to push coverage to codecov.

The base coverage right now as of generating this commit is 54% overall,
missing 300 branches and partially covering 20 other branches
Readme file says python 3.9, so testing on the reported version makes more sense than testing on an older version
- Tests badge in README.md
- Minor cleanups to tests
- Rebuild poetry lock file
These were created separately to make sure that if adding coverage broke tests (or formatting), it would be easier to determine the cause
@Bluesy1 Bluesy1 force-pushed the coverage_testing branch from cd315db to f2cb605 Compare June 16, 2023 01:09
@Bluesy1 Bluesy1 marked this pull request as ready for review June 16, 2023 01:12
@Bluesy1
Copy link
Collaborator Author

Bluesy1 commented Jun 16, 2023

A few things to note that I didn't put in the pr description - If you do want to use a 3rd party provider for code coverage reporting, I'm not familiar with doing it in org owned repos, so I can't help there very much.

But, this should give a pretty reasonable code formatting standard, and base coverage testing setup, and I'm happy to make any changes you think should be made, and/or remove any changes you think are not worth it.

@Bluesy1 Bluesy1 requested a review from firasm June 16, 2023 01:15
@Bluesy1 Bluesy1 force-pushed the coverage_testing branch from 84b3493 to d1c8825 Compare July 4, 2023 01:36
report failures, not just xpasses and xfails
@Bluesy1 Bluesy1 force-pushed the main branch 2 times, most recently from 5d27a03 to 5e18acf Compare June 19, 2024 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Solidify Tests and Code Consistency

2 participants