bpo-25750: add testcase on bad descriptor __get__()

[jdemeyer]

This adds a testcase for bpo-25750 (#6118). This is a separate pull request since the testcase is possibly controversial.

https://bugs.python.org/issue25750

[vstinner]

Hi @serhiy-storchaka, in PR #6118 I wrote that I wasn't really happy about adding an unit test on such really tiny corner case. What do you think? Is it ok to add such test? The unit test is indirectly restricted to CPython since it requires _testcapi.

[vstinner]

@jdemeyer: since I merged your PR #6118, I get that you have now to rebase your PR on top of master to get your fix (to fix the CI).

[jdemeyer]

The unit test is indirectly restricted to CPython since it requires _testcapi.

If that's a problem, I could do

try:
    from _testcapi import bad_get
except ImportError:
    def bad_get(self, obj, cls):
        cls()
        return repr(self)

[encukou]

NEWS entry is already there (from #6118).

Even if it's an edge case, I think the regression test should be merged.

[vstinner]

@encukou: Would you just mind to approve the change in that case?

@serhiy-storchaka: What do you think?

I don't have an opinion strong enough against this change to block it. @encukou: if you want to, please go ahead :-)

[berkerpeksag]

We can wrap the test with @cpython_only.

[vstinner]

I wanted to propose the same change, but IMHO @unittest.skipIf(_testcapi is None, ...) is fine since _testcapi is specific to CPython and it's a common pattern.

[serhiy-storchaka]

cpython_only serves also documenting purpose. It is common to decorate all tests that use _testcapi with cpython_only.

[vstinner]

@jdemeyer: Would you mind to add @cpython_only? IMHO it's better to add it first, before "_testcapi is None".

[jdemeyer]

Would you mind to add @cpython_only? IMHO it's better to add it first, before "_testcapi is None".

Do we really need both? Doesn't @cpython_only imply that _testcapi is available?

[serhiy-storchaka]

I don't have opinion about adding this test. Technically it looks correct.

[serhiy-storchaka]

cpython_only serves also documenting purpose. It is common to decorate all tests that use _testcapi with cpython_only.

[vstinner]

I was the one who blocked the addition of the test. It seems like everybody likes the test, there was just a minor discussion on the @cpython_only decorator. I dislike seeing this PR stuck for such non-blocker issue, so I decided to merge it.

Thanks @jdemeyer for this test. Sorry for being so pendantic :-)

[vstinner]

I propose to not backport this new test to stable Python version. IMHO this test is borderline, I prefer to see how it goes in Python 3.8 first.