Author martin.panter
Recipients Alex.Willmer, Cornelius Diekmann, chris.torek, martin.panter, vstinner
Date 2017-01-04.21:04:51
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1483563891.66.0.957176500318.issue29070@psf.upfronthosting.co.za>
In-reply-to
Content
Hi Cornelius and thanks for the work. Since the patch adds a significant amount of code, I think you might have to sign the contributor agreement: <http://www.python.org/psf/contrib/contrib-form/>. You can do it in a web browser.

I would like to review your tests, but because there is a lot of code to understand and I don’t have much time, it might take me a while. If you can find any way to simplify it, that would be great.

I have some comments on the code review, and will probably post more as I begin to understand what you are proposing.

It looks like you depend on fixing Issue 26228, but the patch there will conflict with your changes. Maybe merge with the other patch, or propose an alternative fix.

The documentation currently mentions the code is only tested on Linux, so it would be nice to update that.

The patch does introduce behavioural changes, if you consider abuse like monkey-patching os.exec() after importing the pty module. It is best not to make unnecessary changes in a bug fix.

Why does the patch slow the tests down so much? Ideally, it is nice to keep the tests as fast as possible.

What is the problem with using a genuine exec() call? Why do we need PtyMockingExecTestBase?
History
Date User Action Args
2017-01-04 21:04:51martin.pantersetrecipients: + martin.panter, vstinner, Alex.Willmer, chris.torek, Cornelius Diekmann
2017-01-04 21:04:51martin.pantersetmessageid: <1483563891.66.0.957176500318.issue29070@psf.upfronthosting.co.za>
2017-01-04 21:04:51martin.panterlinkissue29070 messages
2017-01-04 21:04:51martin.pantercreate