-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add undocumented methods and make types more specific in 2/unittest #3550
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
Conversation
srittau
left a comment
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, a few remarks below.
| def __init__(self, stream: TextIO, descriptions: bool, verbosity: int) -> None: ... | ||
| def getDescription(self, test: TestCase) -> str: ... # undocumented | ||
| def printErrors(self) -> None: ... # undocumented | ||
| def printErrorList(self, flavour: str, errors: List[Tuple[TestCase, str]]) -> None: ... # undocumented |
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.
Looking at the implementation, we can use Iterable instead of List 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.
The implementation calls it with TestResult.errors (List) as an argument. Making it List allows implementations in subclasses to assume the argument is a list, which I think is more useful given that it's routinely called by printErrors and there's not much reason to call it from the outside.
| Optional[BaseException], | ||
| Optional[types.TracebackType]] | ||
|
|
||
| class Testable(metaclass=ABCMeta): |
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.
Do we even need this class? It does not exist in the implementation and I don't really know what purpose it serves in the stubs, especially now that you removed its uses.
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.
It effectively means Union[TestCase, TestSuite]. I didn't remove all uses, just the ones that I noticed should be TestCase instead.
It shouldn't be a class, but changing that would take a lot more work.
stdlib/2/unittest.pyi
Outdated
|
|
||
| _SysExcInfoType = Tuple[Optional[Type[BaseException]], | ||
| Optional[BaseException], | ||
| Optional[types.TracebackType]] |
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.
Would it make more sense to use Union[Tuple[None, None, None], Tuple[Type[BaseException], BaseException, types.TracebackType]]?
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.
You're right, that matches sys.exc_info(). I copied it from the Python 3 stubs, I'll change it there too.
|
I'm trying to import a new version of typeshed into Google, and this change is causing new type errors in Python 2. What's happening is that we have code calling In the unittest implementation, In unittest.pyi, It seems to me that the type definition of |
|
(2) sounds right to me; if it inherits from TestCase at runtime the stub should also inherit from TestCase. |
|
Sounds good, I'll make a PR. |
This makes the pyi file match the implementation: https://github.com/python/cpython/blob/249706c1fbce04125d81bd9993e6c010ae30f8e4/Lib/unittest/case.py#L1019. I also removed a now-redundant `run` method from FunctionTestCase because the mypy test complained about it having a different signature from TestCase.run(). Context: #3550 (comment).
) This makes the pyi file match the implementation: https://github.com/python/cpython/blob/249706c1fbce04125d81bd9993e6c010ae30f8e4/Lib/unittest/case.py#L1019. I also removed a now-redundant `run` method from FunctionTestCase because the mypy test complained about it having a different signature from TestCase.run(). Context: #3550 (comment).
This brings the stubs more in line with the Python 3 stubs and matches the implementation.
TextTestRunner.runis mentioned a few times in the docs but it's not listed separately so I think it's technically undocumented.