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 typeguard, use with pytest #316

Closed
wants to merge 1 commit into from

Conversation

c24t
Copy link
Member

@c24t c24t commented Dec 3, 2019

Inspired by #258 (comment). It would be great to use our existing type annotations in tests at runtime. I.e., without annotating the tests themselves. Hopefully this uncovers some false-positive tests.

This uses typeguard to do runtime checking via an import hook, which should earmark all opentelemetry code for checking.

This PR is very experimental, and may not be a good approach. Caveat reviewer!

@c24t c24t added the WIP Work in progress label Dec 3, 2019
@lzchen
Copy link
Contributor

lzchen commented Dec 4, 2019

This is awesome! I get a lot of errors from typeguard when I run this even on a small subset of the tests (metrics haha). How should we slowly enforce this as to not block and fail the CI, or not having to perform a mass fixing of the tests?

@c24t
Copy link
Member Author

c24t commented Dec 5, 2019

How should we slowly enforce this as to not block and fail the CI, or not having to perform a mass fixing of the tests?

If we do go this route, one option is to fix the tests incrementally in this PR. Even after getting all the tests working, there other problems (e.g. spurious failures for context manager type checks) preventing us from merging this PR though.

@Oberon00 Oberon00 changed the title Add typegaurd, use with pytest Add typeguard, use with pytest Dec 5, 2019
@a-feld a-feld removed their request for review December 10, 2019 00:54
@toumorokoshi toumorokoshi self-assigned this May 21, 2020
@toumorokoshi
Copy link
Member

closing for now due to different priorities, we'll bring this back to an issue (#721) for others to take on.

srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants