-
Notifications
You must be signed in to change notification settings - Fork 83
Add googleapis as test fodder #716
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR integrates the googleapis repository as comprehensive test data for mypy-protobuf, significantly expanding test coverage from a few dozen to over 13,000 proto files. The changes address naming conflicts discovered during this integration where proto field names like property and collections clash with Python built-ins and standard library modules. The solution aliases all imported modules with underscore prefixes (e.g., import collections.abc as _collections_abc) and uses builtins.property for the @property decorator to avoid conflicts.
Key Changes:
- Modified import aliasing strategy to prefix all standard library imports with underscores
- Updated field name handling to use
builtins.propertyto avoid conflicts with@propertydecorator - Added googleapis as a third-party test target
- Extended test exclusions and cleanup procedures for the new test data
- Added test proto fields (
property,collections) to validate the fix
Reviewed changes
Copilot reviewed 77 out of 8800 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| run_test.sh | Adds googleapis proto generation, mypy validation, and cleanup steps to test pipeline |
| pyproject.toml | Excludes unittest proto stubs from type checking |
| proto/testproto/test.proto | Adds property and collections fields to test naming conflict resolution |
| proto/google/protobuf/*.proto | Adds Google protobuf test proto files for comprehensive testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0501cc0 to
a315eb8
Compare
a315eb8 to
615267b
Compare
Signed-off-by: Aidan Jensen <aidandj.github@gmail.com>
Signed-off-by: Aidan Jensen <aidandj.github@gmail.com>
Signed-off-by: Aidan Jensen <aidandj.github@gmail.com>
Signed-off-by: Aidan Jensen <aidandj.github@gmail.com>
git-subtree-dir: third_party/googleapis git-subtree-split: 347b0e45a6ec42e183e44ce11e0cb0eaf7f24caa
fe2d1b0 to
cf7009f
Compare
Pulling in the whole googleapis subtree, generating stubs, and running mypy on them.
Found a few things.
Fields named
propertyandcollectionscaused conflicts with@propertyandcollections.abc.Iterable. In theory this means that fields calledbuiltins,typing, or other built in names could cause issues.To work around this I did 2 things.
property->builtins.property, and all the import names aliased.This is similar to how the first party
.pyigenerator does it (from collections.abc import Mapping as _Mapping), but matching that exactly would have required more architectural changes.During this switch, I found out that
flake8does ast parsing on string names, so_typingdoes not get parsed astyping, this caused a few issues, but I added somenoqa's to handle those. I figure most people aren't running flake8 on their generated stubs.Overall the change seems large, but conceptually it's a very minor change. And we're now testing on over 13000 proto files!!!
It also turns out that even though
mypy-protobufreserves field numbers 1151-1154, google itself appears to not respect it. 😬