classification
Title: Use proper command line parsing in _testembed
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: brett.cannon, cstratak, encukou, eric.snow, ncoghlan, python-dev, steve.dower
Priority: normal Keywords: patch

Created on 2015-08-25 05:51 by ncoghlan, last changed 2017-05-09 06:10 by ncoghlan. This issue is now closed.

Files
File name Uploaded Description Edit
24932_1.patch steve.dower, 2016-12-30 18:28 review
Pull Requests
URL Status Linked Edit
PR 552 closed dstufft, 2017-03-31 16:36
Messages (9)
msg249106 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-08-25 05:51
Programs/_testembed (invoked by test_capi to test CPython's embedding support) is currently a very simple application with only two different embedding tests: https://hg.python.org/cpython/file/tip/Programs/_testembed.c

In light of proposals like PEP 432 to change the interpreter startup sequence and make it more configurable, it seems desirable to be better able to test more configuration options directly, without relying on the abstraction layer provided by the main CPython executable.

The specific unit testing library that prompted this idea was cmocka, which is used by libssh, sssd and cwrap: https://cmocka.org/

cwrap in turn is used by the Samba team to mock out network interfaces and other operations using LD_PRELOAD: https://cwrap.org/

We don't necessarily have to use those particular libraries, I'm just filing this issue to capture the idea of improving our C level unit testing capabilities, and then updating regrtest to better capture and report those results (currently there are just a couple of tests in test_capi that call the _testembed executable)
msg249134 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-08-25 16:24
Someone is going to think of [googletest] (https://github.com/google/googletest) and then realize that it is a C++ test suite and thus won't work unless you explicitly compile Python for C++.
msg284338 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2016-12-30 18:28
The only real advantage of adding a native unit testing framework here is to avoid having to start/destroy _testembed[.exe] multiple times during the test run. But given the nature of these tests is highly environmental, I don't think we can reasonably avoid it. I'm also highly doubtful that any framework is going to actually reduce the work we'd need to do to mock out initialization steps.

I've attached a patch that takes the first step towards making _testembed more usable, by changing main() to look up a test from a list and execute it, then return the exit code. The support in test_capi.EmbeddingTests is neat, so adding more tests here will be fairly straightforward.
msg284360 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-12-31 04:08
Steve's patch LGTM, and I now agree our needs are simple enough that we can rely on subprocess+unittest for the result reporting.

I'd just missed Steve's simple solution of using the return code to indicate success or failure, as well as Eric's approach in issue 29102 of teaching the related Python test case to read the text output from the _testembed test case.

If we eventually want to go beyond the single "run this test case" argument, then adding getopt() support to _testembed should be sufficient.
msg284456 - (view) Author: Roundup Robot (python-dev) Date: 2017-01-02 04:25
New changeset 69b2a6284f3d by Steve Dower in branch 'default':
Issue #24932: Use proper command line parsing in _testembed
https://hg.python.org/cpython/rev/69b2a6284f3d
msg284457 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2017-01-02 04:27
Patch committed without modification. I'll keep an eye on the buildbots just in case, though I may end up going offline before they get to this changeset.
msg284468 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2017-01-02 06:12
Buildbots are happy, so I'm calling this done. Jumping straight to getopt is overengineering right now in my opinion, but if there's a need for it with a future test we can consider it then.
msg293110 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2017-05-05 14:50
Downstream backporting of PEP 538 to the 3.6 branch also depends on this fix.

Would it be beneficial in any way to cherry-pick it for 3.6?
msg293281 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-05-09 06:10
Based on the latest round of PEP 538 review, the related test case that currently relies on _testembed is going to be able to be simplified to just setting LC_ALL=C when calling the subprocess (the amount of code it's letting me delete from the draft patch is further persuading me that changing the env var updates from "LC_ALL & LANG" to "LC_CTYPE & LANG" is the right call).

That means that change will also eliminate the dependence on this test suite enhancement for backports.
History
Date User Action Args
2017-05-09 06:10:51ncoghlansetmessages: + msg293281
2017-05-05 14:50:32cstrataksetnosy: + cstratak
messages: + msg293110
2017-03-31 16:36:23dstufftsetpull_requests: + pull_request965
2017-01-02 06:12:48steve.dowersetstatus: open -> closed
resolution: fixed
messages: + msg284468

stage: commit review -> resolved
2017-01-02 04:27:09steve.dowersetassignee: steve.dower
messages: + msg284457
stage: commit review
2017-01-02 04:25:37python-devsetnosy: + python-dev
messages: + msg284456
2016-12-31 04:08:21ncoghlansetmessages: + msg284360
2016-12-31 04:01:03ncoghlansettitle: Migrate _testembed to a C unit testing library -> Use proper command line parsing in _testembed
2016-12-30 18:28:40steve.dowersetfiles: + 24932_1.patch
versions: + Python 3.7
nosy: + steve.dower

messages: + msg284338

keywords: + patch
2015-08-25 16:24:50brett.cannonsetnosy: + brett.cannon
messages: + msg249134
2015-08-25 05:51:28ncoghlancreate