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

Roundup Robot added the comment: New changeset 3bd4bed78a0b068e28bcf2242d33aed227c2532c by Michael Foord (Miss Islington (bot)) in branch '3.8': bpo-34596: Fallback to a default reason when @unittest.skip is uncalled (GH-9082) (#15781)

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

Change by Michael Foord : -- assignee: -> michael.foord components: +Extension Modules -Library (Lib), Tests resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

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

Change by miss-islington : -- pull_requests: +15434 pull_request: https://github.com/python/cpython/pull/15781 ___ Python tracker ___

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

Roundup Robot added the comment: New changeset d5fd75c53fad7049fc640c9a6162d35f0c5bea03 by Michael Foord (Naitree Zhu) in branch 'master': bpo-34596: Fallback to a default reason when @unittest.skip is uncalled (#9082)

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

Michael Foord added the comment: I'm in favour of a default and "Unconditionally skipped" is fine with me. Although "Skipped" would also be fine. Making @skip work with no arguments is fine. Having to pass in arguments message arguments you don't want is a pain and there's no need to force

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

Zachary Ware added the comment: I think we need Michael's ruling on which way to go here. I'm fine with either an exception or a default reason, but still lean towards the default. I don't think `skipIf` or `skipUnless` really need a change either way; they both require two arguments and

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

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 `@skipUnless`

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

Berker Peksag added the comment: It would be nice to make *reason* optional. Every time I needed to use @unittest.skip() (even if I wanted to use it temporarily), I ended up passing some random string as reason or use 'raise SkipTest' directly. If we decide to keep *reason* required, I

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

Steven D'Aprano added the comment: For what its worth, I'm +1 with Serhiy that we raise an exception on a non-string argument, including the case where the caller forgets to provide a reason at all. I don't think that @skip with no reason was ever intended to work (it certainly wasn't

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

Zachary Ware added the comment: I don't agree that this change makes the implementation significantly more cumbersome. I also think there's a backward compatibility argument to be made for allowing the uncalled usage, particularly considering the OP's published example and other similar

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

Serhiy Storchaka added the comment: It doesn't look like a good design to me if allow a function be a decorator and a decorator fabric at the same time. It always lead to cumbersome and errorprone implementation. Currently there is only one example of such design in the stdlib, other

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

Change by Roundup Robot : -- keywords: +patch pull_requests: +8540 stage: -> patch review ___ Python tracker ___ ___

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

Naitree Zhu added the comment: Ok, I see. Thank you. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

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

Change by Zachary Ware : -- nosy: +ezio.melotti, michael.foord, rbcollins ___ Python tracker ___ ___ Python-bugs-list mailing list

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

Zachary Ware added the comment: I think we should make the same change on all branches (why would we fix it in maintenance branches just to break it in the next major release?), in which case it's just one PR to master and our backport bot (and/or the merging core dev) will take care of it

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

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 PRs to

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

Zachary Ware added the comment: "Unconditionally" :) -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

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

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

Zachary Ware added the comment: It could be interesting to enable uncalled `skip` by setting a default reason of "Unconditional skip" when the argument is a function. Do note that decorating with an uncalled `skip` does actually work to skip the test currently, but the test is marked as

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

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

Steven D'Aprano added the comment: Is there a use-case for reason to be anything but a string? -- nosy: +steven.daprano ___ Python tracker ___

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

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