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

Add per-package coverage enforcement mechanism #39339

Open
scbedd opened this issue Jan 22, 2025 · 2 comments
Open

Add per-package coverage enforcement mechanism #39339

scbedd opened this issue Jan 22, 2025 · 2 comments
Labels
EngSys This issue is impacting the engineering system.

Comments

@scbedd
Copy link
Member

scbedd commented Jan 22, 2025

@pdhotems has requested that we enforce a minimum code coverage number, below which we will deny the pullrequest.

I didn't receive any specific feedback on the feature itself. I think the way I'd want this to work is to offer a pyproject.toml customization that will enable this behavior. That way nobody will be surprised.

Here is a section of the azure-ai-ml pyproject.toml

# existing config
[tool.azure-sdk-build]
mypy = true
pyright = false
type_check_samples = false
pylint = true
mindependency = false
latestdependency = false
black = true
# new keys
enforce_coverage = true
coverage_limit = 0.3 # the check will be <calculated_cov_%> <= <coverage_limit>

Where the overall coverage goal comes from coverage_limit, and enforce_coverage enables this behavior. Opt-in to start.

@scbedd scbedd added the EngSys This issue is impacting the engineering system. label Jan 22, 2025
@pdhotems
Copy link
Member

pdhotems commented Jan 22, 2025

Thanks, Scott, for the suggestion. I would be glad if we could prioritize this and we would be happy to contribute.

Many issues that we have come across recently for the azure-ai-ml package could have been avoided with better test coverage. The code coverage for the package has dropped significantly in recent times, and we are working to improve this. However, since this package is owned by multiple teams, it is important to set a code coverage threshold in the pipeline to reject pull requests that do not meet the standard.

@scbedd
Copy link
Member Author

scbedd commented Jan 22, 2025

I notified the python team of this request, and @lmazuel mentioned a couple items to be aware of / inform implementation of this feature.

Coverage as an absolute score. What is the right value? 50? 60? 80? What is the current score globally.
Coverage diff must be positive. IOW, whatever your current score, new code should not decrease score. This forces a more gentle story, where you can only improve
I would start with the diff one, before the absolute one, but if they feel for both we can discuss a knob in pyproject.toml for insance

For the typing score, with Krista we did a diff with a buffer of 5%, and the actual score is not enforced (but encouraged). I would start with that, and add the score to the dashboard

So the biggest thing right now is the changes to the pipelines to

  • A) track coverage on a per-package basis
  • B) provide a way to break if coverage drops from build - 1.

The individual knobs in the pyproject.toml will be optional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EngSys This issue is impacting the engineering system.
Projects
None yet
Development

No branches or pull requests

2 participants