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

73: update .toml to enable pytest-cov report #74

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

John-Sharples
Copy link
Contributor

@John-Sharples John-Sharples commented Oct 16, 2023

In this PR some general tidy up of the existing github action:

  • update branches that trigger actions to include any that start with an issue number and a dash ( e.g. 14-some-string)
  • Change unit test job to include coverage report.
  • move pytest/coverage config items to pyproject.toml this fixes the current error in actions where coverage can't find a test report.
  • Above change also introduces a coverage check. Pytest will fail if coverage drops below 98%. Do we want this?
  • Add h5netcdf to dev requirements, it's needed to run tests (specifically test_murphy.py)

I'll keep issue #73 open, as there is more to do. But this PR can go in now as these changes will be usful for everyone.

@John-Sharples
Copy link
Contributor Author

See here for example output https://github.com/John-Sharples/scores/actions/runs/6540346623

@tennlee
Copy link
Collaborator

tennlee commented Oct 17, 2023

This looks great. I think we should consider whether 98 to 100 is what should be required for coverage (I would probably vote for 100 with in-line nocov comments with an explanation), but the change looks very helpful as-is.

@tennlee tennlee closed this Oct 17, 2023
@tennlee tennlee reopened this Oct 17, 2023
@tennlee tennlee merged commit fb5b160 into nci:develop Oct 17, 2023
6 checks passed
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.

2 participants