-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Increase test coverage a bit #8015
Conversation
please review |
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.
Thanks! Test additions look very welcome, just wondering about the justification for the pragmas.
@@ -130,7 +129,7 @@ def for_model(cls, bases: tuple[type[Any], ...], namespace: dict[str, Any], kwar | |||
return cls(config_new) | |||
|
|||
# we don't show `__getattr__` to type checkers so missing attributes cause errors | |||
if not TYPE_CHECKING: | |||
if not TYPE_CHECKING: # pragma: no branch |
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's the justification for the pragma here?
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.
coverage says this line is partially covered because the condition is never false. no branch
(not no cover
) says to ignore that, because not TYPE_CHECKING
will never be false at runtime.
@@ -196,7 +196,7 @@ def wrapped_model_post_init(self: BaseModel, __context: Any) -> None: | |||
# this is the BaseModel class itself being created, no logic required | |||
return super().__new__(mcs, cls_name, bases, namespace, **kwargs) | |||
|
|||
if not typing.TYPE_CHECKING: | |||
if not typing.TYPE_CHECKING: # pragma: no branch |
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.
Similar question here.
Change Summary
A few small tweaks and additions to tests to increase overall test coverage.
Related issue number
Part of #7656
Checklist
Selected Reviewer: @davidhewitt