Skip to content

gh-110147: run console io test in new process #110268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Oct 5, 2023
49 changes: 30 additions & 19 deletions Lib/test/test_msvcrt.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import os
import subprocess
import sys
import unittest
from textwrap import dedent

from test.support import os_helper
from test.support import os_helper, requires_resource
from test.support.os_helper import TESTFN, TESTFN_ASCII

if sys.platform != "win32":
raise unittest.SkipTest("windows related tests")

import _winapi
import msvcrt;

from _testconsole import write_input, flush_console_input_buffer
import msvcrt


class TestFileOperations(unittest.TestCase):
Expand Down Expand Up @@ -61,34 +61,45 @@ def test_get_osfhandle(self):


class TestConsoleIO(unittest.TestCase):
# CREATE_NEW_CONSOLE creates a "popup" window.
@requires_resource('gui')
def run_in_separated_process(self, code):
# Run test in a seprated process to avoid stdin conflicts.
# See: gh-110147
cmd = [sys.executable, '-c', code]
subprocess.run(cmd, check=True, capture_output=True,
creationflags=subprocess.CREATE_NEW_CONSOLE)

def test_kbhit(self):
h = msvcrt.get_osfhandle(sys.stdin.fileno())
flush_console_input_buffer(h)
self.assertEqual(msvcrt.kbhit(), 0)
code = dedent('''
import msvcrt
assert msvcrt.kbhit() == 0
''')
self.run_in_separated_process(code)

def test_getch(self):
msvcrt.ungetch(b'c')
self.assertEqual(msvcrt.getch(), b'c')

def test_getwch(self):
with open('CONIN$', 'rb', buffering=0) as stdin:
h = msvcrt.get_osfhandle(stdin.fileno())
flush_console_input_buffer(h)
def check_getwch(self, funcname):
code = dedent(f'''
import msvcrt
from _testconsole import write_input
with open("CONIN$", "rb", buffering=0) as stdin:
write_input(stdin, {ascii(c_encoded)})
assert msvcrt.{funcname}() == "{c}"
''')
self.run_in_separated_process(code)

write_input(stdin, c_encoded)
self.assertEqual(msvcrt.getwch(), c)
def test_getwch(self):
self.check_getwch('getwch')

def test_getche(self):
msvcrt.ungetch(b'c')
self.assertEqual(msvcrt.getche(), b'c')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be safer to run this test in a subprocess with a new console, no?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it would, but it's annoying. We might want to put it behind a resource marker, as the new console will pop up new GUI when it runs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would CREATE_NO_WINDOW flag prevent popups?

The process is a console application that is being run without a console window. Therefore, the console handle for the application is not set. This flag is ignored if the application is not a console application, or if it is used with either CREATE_NEW_CONSOLE or DETACHED_PROCESS.

says: https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems it should work, I'll try it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the console handle for the application is not set

Means no console and the test is irrelevant.

ignored if ... used with either CREATE_NEW_CONSOLE

This is the flag that you'd use to make the test more reliable, and it's incompatible with CREATE_NO_WINDOW.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, and added the @requires_resource('gui') marker to avoid the console pop-up.

For the tests test_getche and test_getch, they uses msvcrt.ungetch instead of the magic write_input, I think this stuff is stable enough, this test file is added and disabled days ago, so I fixed it serveral times, and never saw them failed once. So I think we should keep it (test_getche / test_getch) as current.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the tests test_getche and test_getch, they uses msvcrt.ungetch instead of the magic write_input, I think this stuff is stable enough

Hum, ok. Let's see if CI will agree with you :-) I'm fine with keeping them in the same process for now.


def test_getwche(self):
with open('CONIN$', 'rb', buffering=0) as stdin:
h = msvcrt.get_osfhandle(stdin.fileno())
flush_console_input_buffer(h)

write_input(stdin, c_encoded)
self.assertEqual(msvcrt.getwche(), c)
self.check_getwch('getwche')

def test_putch(self):
msvcrt.putch(b'c')
Expand Down
19 changes: 0 additions & 19 deletions PC/_testconsole.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,31 +133,12 @@ _testconsole_read_output_impl(PyObject *module, PyObject *file)
Py_RETURN_NONE;
}

/*[clinic input]
_testconsole.flush_console_input_buffer
handle: HANDLE

Flushes the console input buffer.

All input records currently in the input buffer are discarded.
[clinic start generated code]*/

static PyObject *
_testconsole_flush_console_input_buffer_impl(PyObject *module, void *handle)
/*[clinic end generated code: output=1f923a81331465ce input=be8203ae84a288f5]*/
/*[clinic end generated code:]*/
{
FlushConsoleInputBuffer(handle);

Py_RETURN_NONE;
}

#include "clinic\_testconsole.c.h"

PyMethodDef testconsole_methods[] = {
_TESTCONSOLE_WRITE_INPUT_METHODDEF
_TESTCONSOLE_READ_OUTPUT_METHODDEF
_TESTCONSOLE_FLUSH_CONSOLE_INPUT_BUFFER_METHODDEF
{NULL, NULL}
};

Expand Down
70 changes: 1 addition & 69 deletions PC/clinic/_testconsole.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.