-
Notifications
You must be signed in to change notification settings - Fork 197
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
Enforce 95% unit test coverage for new code #1112
base: mainline
Are you sure you want to change the base?
Conversation
0fb3fdf
to
64822ff
Compare
28dcf64
to
fb7ecb2
Compare
bb7f17a
to
0ecc5f5
Compare
0ecc5f5
to
b59d44c
Compare
pytest --durations=100 --cov=src --cov-branch --cov-context=test \ | ||
--cov-report=html:cov_html --cov-report=xml:cov.xml --cov-report term:skip-covered \ | ||
--md-report --md-report-flavor gfm --md-report-output pytest_result_summary.md \ | ||
tests/unit_tests/ | tee pytest_output.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the plan going forward? Will we set separate coverage threshold for unit tests and integration tests? Or do we combine the coverage report and set one threshold for all the tests.
if: github.event_name == 'pull_request' | ||
run: | | ||
cd marqo | ||
diff-cover cov.xml --compare-branch=origin/${{ github.base_ref }} --fail-under=95 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If unit test coverage is enforced to be above 95, do we still need to keep the same check in unit_test_200gb_CI workflow?
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Have unit tests been run against this PR? (Has there also been any additional testing?)
Related Python client changes (link commit/PR here)
Related documentation changes (link commit/PR here)
Other information:
Please check if the PR fulfills these requirements