This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: test_capi test fails because of mismatched newlines
Type: Stage: resolved
Components: Tests Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: christian.heimes, giampaolo.rodola, ncoghlan, python-dev, r.david.murray, steve.dower, vstinner
Priority: normal Keywords: patch

Created on 2014-10-26 20:28 by steve.dower, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
test_capi.patch steve.dower, 2014-10-26 20:28 review
Messages (13)
msg230034 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2014-10-26 20:28
The test_capi.test_forced_io_encoding test currently requires that newlines match when checking the results. This apparently does not occur with VC10 builds, but does appear with newer versions of the compiler.

This patch normalises the line endings in the values being checked. Just seeking a quick review before I check it in - not sure who's best to nosy, so I picked the test coverage group.
msg230038 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-26 20:41
I don't understand why this patch is necessary.  The test already joins the expected output with os.linesep (and doesn't test against err, that's just for verbose informational purposes).  Are you saying that the generated output is not using os.linesep?  In that case wouldn't that be a bug?
msg230042 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2014-10-26 20:54
The generated output is using os.linesep, but the result from run_embedded_interpreter() is not.

I assume there's a difference (fix) in MSVCRT between VC10 and VC12 (maybe VC11 - didn't test) that causes '\n' to flow through Popen.communicate() where it previously was converted to '\r\n'.
msg230055 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-27 01:43
That's my point.  What comes out of communicate *should* be os.linesep, unless I'm missing something.  So this test failing indicates a bug.  Of course, I've never worked with an embedded interpreter, so there could be something I'm missing.  But, you say yourself it is a behavior change, and that in itself is a potential bug.
msg230070 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2014-10-27 11:58
Argh, you're making me page _testembed back into my brain. I try to avoid having to do that ;)

Anyway, this doesn't look like the right fix to me - although it may indeed be a test bug uncovered by a VC10->VC14 behavioural change in the behaviour of printf().

The origin of the output being checked is https://hg.python.org/cpython/file/default/Programs/_testembed.c#l79

Note the mixture of output from C level printf() calls and Python level print() calls inside check_stdio_details().

My guess would be that VC10 is translating '\n' to '\r\n' in the printf() calls, and VC14 has stopped doing that.

To confirm my theory: check if it is only the lines that start with "Expected" that end with '\n' rather than '\r\n' under VC14 (those are the ones produced directly from C - the others are produced via Python's print builtin).

If that *is* what's happening, we may want to convert the embedding tests over to running the subprocess in universal newlines mode, and adjust the expected output accordingly.
msg230093 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2014-10-27 19:16
> Argh, you're making me page _testembed back into my brain. I try to avoid having to do that ;)

Hehe, sorry.

> My guess would be that VC10 is translating '\n' to '\r\n' in the
> printf() calls, and VC14 has stopped doing that.
>
> To confirm my theory: check if it is only the lines that start 
> with "Expected" that end with '\n' rather than '\r\n' under VC14
> (those are the ones produced directly from C - the others are
> produced via Python's print builtin).

Confirmed. Enabling universal_newlines and using '\n'.join() instead of os.linesep.join() for the expected result works.

Does that sound like it would be the correct fix? Or is the printf() change something that we should try and keep consistent with VC10?
msg230206 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2014-10-29 12:24
Yes, switching the subprocess invocation in test_capi.py to universal newlines mode and joining the expected output with '\n' sounds like the right test case fix to me.

As far as the printf() change goes, I'm not really the right person to ask - all my Windows dev experience is with Python and C++, while my C dev experience has been primarily on TI DSP's and Linux.

If I had to guess, the behavioural change is likely a result of the C99 enhancements and standards conformance improvements described in http://blogs.msdn.com/b/vcblog/archive/2014/06/03/visual-studio-14-ctp.aspx
msg230207 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2014-10-29 12:28
One last thing: we may also want to update the C API docs to explicitly point out the discrepancy in handling of '\n' on Windows between printf() in VC14+ (which no longer does the '\r\n' substitution) and the print functions in the Python C API (which will keep the transformation).

This is a weird enough quirk that I feel like we should explicitly point it out *somewhere*, but also obscure enough that I don't want to bother people with it unnecessarily.
msg230210 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-29 14:25
That information should also go in the porting notes section of the 3.5 What's New, but clearly labeled (if I understand correctly) as applying only to applications that embed python on Windows.
msg230212 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2014-10-29 15:09
I think it's also in any extension that prints directly to the console without going through the Python IO machinery.

I'll make the test change in default, since that's what the os.linesep is trying to achieve anyway, and modify the porting notes in my branch, since it won't apply until the build changes are merged in.

Thanks all.
msg230463 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-11-01 22:11
New changeset edb270e5c9c3 by Steve Dower in branch 'default':
#22731 test_capi test fails because of mismatched newlines
https://hg.python.org/cpython/rev/edb270e5c9c3
msg230472 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2014-11-01 23:54
As near as I can tell, Py_Initialize() is causing the stdout mode to change from O_TEXT to O_BINARY in _testembed.

Looking at the output in this test, you can see the first printf('\n') (after 'Use defaults') is correctly translated, but the next '\n' comes after Py_Initialize().

b'--- Use defaults ---\r\n... \r\n--- Set encoding only ---\n...'

I don't really see how this can be avoided without breaking something else to do with text IO. Probably the documentation for Py_Initialize should get the note, rather than the What's New section. Py_Initialize has really brief docs, and I'm certain there are plenty of other little things like this that change when you call it - is this the right place for compiler-specific caveats like this? I don't really know where else it could go. Or is it something that we really do want to fix? (We generally avoid changing global settings like this where possible.)
msg275383 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-09 18:48
New changeset b31e803d5ad3 by Steve Dower in branch '3.5':
Closes #22731: Documents change of console mode.
https://hg.python.org/cpython/rev/b31e803d5ad3

New changeset e93a8088a1f9 by Steve Dower in branch 'default':
Closes #22731: Documents change of console mode.
https://hg.python.org/cpython/rev/e93a8088a1f9
History
Date User Action Args
2022-04-11 14:58:09adminsetgithub: 66920
2016-09-09 18:48:06python-devsetstatus: open -> closed

messages: + msg275383
2014-11-01 23:54:40steve.dowersetstatus: closed -> open

messages: + msg230472
2014-11-01 22:12:05steve.dowersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2014-11-01 22:11:39python-devsetnosy: + python-dev
messages: + msg230463
2014-10-29 15:09:18steve.dowersetmessages: + msg230212
2014-10-29 14:25:20r.david.murraysetmessages: + msg230210
2014-10-29 12:28:56ncoghlansetmessages: + msg230207
2014-10-29 12:24:10ncoghlansetmessages: + msg230206
2014-10-27 19:16:22steve.dowersetmessages: + msg230093
2014-10-27 12:51:37vstinnersetnosy: + vstinner
2014-10-27 11:58:02ncoghlansetmessages: + msg230070
2014-10-27 01:43:43r.david.murraysetmessages: + msg230055
2014-10-26 20:54:19steve.dowersetmessages: + msg230042
2014-10-26 20:41:23r.david.murraysetnosy: + r.david.murray
messages: + msg230038
2014-10-26 20:28:43steve.dowercreate