[issue34596] [unittest] raise error if @skip is used with an argument that looks like a test method

2018-09-14 Thread Naitree Zhu
Naitree Zhu added the comment: Hi guys, what's our consensus on this? - raise an exception as a fix? or - fallback to default `reason` as a new feature? If we choose to explicitly make `reason` optional (I mean by documenting it as such), shouldn't we also change `@skipIf` and `@

[issue34596] [unittest] raise error if @skip is used with an argument that looks like a test method

2018-09-06 Thread Naitree Zhu
Naitree Zhu added the comment: Ok, I see. Thank you. -- ___ Python tracker <https://bugs.python.org/issue34596> ___ ___ Python-bugs-list mailing list Unsub

[issue34596] [unittest] raise error if @skip is used with an argument that looks like a test method

2018-09-06 Thread Naitree Zhu
Naitree Zhu added the comment: Hi @zach.ware, Just to make sure I'm getting this right (first time contributing to cpython...) Now I need to open 4 PRs at GitHub, - 1 PR to master branch, with following changes: raise TypeError when `reason` is not a string. (Include unit test.) - 3 P

[issue34596] [unittest] raise error if @skip is used with an argument that looks like a test method

2018-09-06 Thread Naitree Zhu
Naitree Zhu added the comment: What would be a good default reason? How about the function name? if isinstance(reason, types.FunctionType): reason = reason.__name__ For example, from unittest import TestCase, skip class Test(TestCase): @skip def

[issue34596] [unittest] raise error if @skip is used with an argument that looks like a test method

2018-09-06 Thread Naitree Zhu
Naitree Zhu added the comment: Well, I personally can not think of any. I think `reason` should normally just be string. If that is ok, I'll be happy to submit a PR that restricts reason to be a string. -- ___ Python tracker &

[issue34596] [unittest] raise error if @skip is used with an argument that looks like a test method

2018-09-06 Thread Naitree Zhu
New submission from Naitree Zhu : When using @skip decorator, `reason` argument is required. But one could easily overlook that and use it like so: class SomeTest(TestCase): @skip def test_method(self): # ... The test actually passes when running with