| msg201379 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2013-10-26 17:41 |
| It is *not* OK to have a "test.support.HAVE_DOCSTRINGS" flag that is true under -OO, nor a requires_docstrings decorator that still attempts to run the test under those conditions. Issue 19330 updated them so their meaning matched their names and the surrounding comments, but they were apparently being used instead to refer to a specific kind of CPython build. A more logical name for such a feature would be HAVE_C_DOCSTRINGS and requires_c_docstrings. |
|
|
| msg201380 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-26 17:51 |
| Alternatively, if the affected tests should also be skipped under -OO, then I think changing the current definition of HAVE_DOCSTRINGS would also work: MISSING_C_DOCSTRINGS = (check_impl_detail() and sys.platform != 'win32' and not sysconfig.get_config_var('WITH_DOC_STRINGS')) HAVE_DOCSTRINGS = _check_docstrings.__doc__ is not None and not MISSING_C_DOCSTRINGS |
|
|
| msg201381 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2013-10-26 17:53 |
| Think this is related to why the FreeBSD default buildbot is always failing now? http://buildbot.python.org/all/builders/AMD64%20FreeBSD%209.0%203.x/builds/5619/steps/test/logs/stdio Like: File "/usr/home/buildbot/buildarea/3.x.krah-freebsd/build/Lib/test/test_generators.py", line ?, in test.test_generators.__test__.email Failed example: print(i.__next__.__doc__ if HAVE_DOCSTRINGS else 'x.__next__() <==> next(x)') Expected: x.__next__() <==> next(x) Got: I have no idea what HAVE_DOCSTRINGS is all about, alas :-( |
|
|
| msg201385 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-10-26 17:59 |
| Yeah, HAVE_DOCSTRINGS was added when we were not sure whether to skip C and Python docstrings separately. I agree that the right thing is to have both requires_c_docstrings and requires_py_docstrings. |
|
|
| msg201386 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-10-26 18:05 |
| For reference see #17041 and . Separating the decorators makes things clearer (after all we're dealing with extremely obscure features here), merging them could mean less work. |
|
|
| msg201387 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-26 18:08 |
| +1 for separate flags with appropriate names and skip messages. It should only be the usage in test_contextlib which needs to be migrated to the Python flags, although the C usage may be more widespread. (The other Python cases are currently checking the optimisation level instead) However, it's 4 am here, so I'm going to go get some sleep :) |
|
|
| msg201406 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-27 02:46 |
| I'll do an initial quick fix that also turns off HAVE_DOCSTRINGS when C level docstrings are missing, before working on a large patch to split it into two separate flags. |
|
|
| msg201412 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-10-27 04:19 |
| New changeset 1927b7c01c78 by Nick Coghlan in branch 'default': Mitigate #19412: restore test skips for --without-doc-strings http://hg.python.org/cpython/rev/1927b7c01c78 |
|
|
| msg201415 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-27 05:39 |
| On my Fedora system, test_gdb still fails when --without-doc-strings is configured, even after updating the decorator to cover both C and Python docstrings as an interim fix. That may be a real bug, though (see issue 19415 for details) Regarding splitting the flags, I'm not sure I agree with RDM's reasoning in issue 17041. As soon as *any* docstrings are missing (whether it's C docstrings or Python docstrings), various parts of the introspection machinery are going to start behaving strangely. By splitting the flags in the test suite, we're increasing the coupling between the tests and the implementation. For example, if something docstring dependent that was previously implemented in Python is moved to a C accelerator module, is it right for the test to have to know about that and switch to moving a different decorator? In my view, "this is a C docstring" and "this is a Python docstring" is an implementation detail that the tests shouldn't have to care about. With the latest update, "test.supports.requires_docstrings" means "I require trustworthy docstrings, and if there's anything wrong with the docstring machinery, don't even try this test" without getting into the specifics of worrying about *why* the docstrings might be unreliable. |
|
|
| msg201421 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-10-27 06:26 |
| I suppose that some tests (e.g. test_genexps.py) still fail with -O2. |
|
|
| msg201474 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-10-27 16:28 |
| Nick: I agree with your reasoning. I don't know why I said what I did on the other issue. |
|
|
| msg201509 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-28 02:07 |
| OK, since David agrees the updated behaviour of the requires_docstring decorator makes sense (i.e. skipping __doc__ related tests whenever the docstrings are unreliable, regardless of the reason), I'm morphing this to be a test.support docs issue. At least requires_docstring should be documented, but it may be worth documenting HAVE_DOCSTRINGS and MISSING_C_DOCSTRINGS as well. |
|
|
| msg203110 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2013-11-17 00:12 |
| I was bitten by this in #19535. See the third point about test_contextlib in . I would expect support.requires_docstring to skip the test if -OO is used and the docstrings are missing. |
|
|
| msg312601 - (view) |
Author: Cheryl Sabella (cheryl.sabella) * |
Date: 2018-02-22 23:06 |
| I added `HAVE_DOCSTRINGS`,`MISSING_C_DOCSTRINGS`, `requires_docstring` to the test.support documentation under . Additional changes to those three have been made under , so I am closing this issue in favor of . Please comment on that PR if the wording still needs to be adjusted. Thanks! |
|
|