From 2d4e6917aca5fdca1a648489f747bdc1fd21352f Mon Sep 17 00:00:00 2001 From: Matiiss <83066658+Matiiss@users.noreply.github.com> Date: Fri, 23 Feb 2024 01:28:05 +0200 Subject: [PATCH 1/4] Fix passing `parent_window=None` to `message_box` --- docs/reST/ref/display.rst | 5 +---- src_c/display.c | 17 +++++++++++------ test/display_test.py | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/docs/reST/ref/display.rst b/docs/reST/ref/display.rst index 6a7b2248be..67e05eb985 100644 --- a/docs/reST/ref/display.rst +++ b/docs/reST/ref/display.rst @@ -816,13 +816,10 @@ required). :param str title: A title string. :param str message: A message string. If this parameter is set to ``None``, the message will be the title. :param str message_type: Set the type of message_box, could be ``"info"``, ``"warn"`` or ``"error"``. + :param Window parent_window: The parent window of the message box. :param tuple buttons: An optional sequence of button name strings to show to the user. :param int return_button: Button index to use if the return key is hit, ``0`` by default. :param int escape_button: Button index to use if the escape key is hit, ``None`` for no button linked by default. -.. - (Uncomment this after the window API is published) - :param Window parent_window: The parent window of the message_box -.. :return: The index of the button that was pushed. diff --git a/src_c/display.c b/src_c/display.c index b899144c63..bbb385dcde 100644 --- a/src_c/display.c +++ b/src_c/display.c @@ -2710,10 +2710,10 @@ pg_message_box(PyObject *self, PyObject *arg, PyObject *kwargs) "parent_window", "buttons", "return_button", "escape_button", NULL}; - if (!PyArg_ParseTupleAndKeywords( - arg, kwargs, "s|OsO!OiO", keywords, &title, &message, &msgbox_type, - &pgWindow_Type, &parent_window, &buttons, &return_button_index, - &escape_button_index_obj)) { + if (!PyArg_ParseTupleAndKeywords(arg, kwargs, "s|OsOOiO", keywords, &title, + &message, &msgbox_type, &parent_window, + &buttons, &return_button_index, + &escape_button_index_obj)) { return NULL; } @@ -2750,10 +2750,15 @@ pg_message_box(PyObject *self, PyObject *arg, PyObject *kwargs) msgbox_data.flags |= SDL_MESSAGEBOX_BUTTONS_LEFT_TO_RIGHT; #endif - if (parent_window == Py_None) + if (parent_window == Py_None) { msgbox_data.window = NULL; - else + } + else { + if (!pgWindow_Check(parent_window)) { + return RAISE(PyExc_TypeError, "'parent_window' must be a Window"); + } msgbox_data.window = ((pgWindowObject *)parent_window)->_win; + } msgbox_data.colorScheme = NULL; // use system color scheme settings diff --git a/test/display_test.py b/test/display_test.py index 27d3c3b154..ecf9a361df 100644 --- a/test/display_test.py +++ b/test/display_test.py @@ -948,6 +948,40 @@ def test_messagebox_args(self): self.assertRaises(TypeError, lambda: mb("", buttons=())) self.assertRaises(TypeError, lambda: mb("", parent_window=123456)) + # the important bit is `parent_window=None` + self.assertRaises(TypeError, lambda: mb("", buttons=(), parent_window=None)) + + @staticmethod + def _create_message_box_in_process(has_reached_message_box_flag): + has_reached_message_box_flag.value = 1 + pygame.display.message_box("", parent_window=None) + + @unittest.skipIf( + sys.platform.startswith("emscripten"), "multiprocessing not available" + ) + def test_message_box_with_parent_window_none(self): + import multiprocessing + + has_reached_message_box = multiprocessing.Value("i", 0) + proc = multiprocessing.Process( + target=self._create_message_box_in_process, args=(has_reached_message_box,) + ) + proc.start() + + # wait until the process actually reaches the line where it calls `message_box` + while not has_reached_message_box.value: + pass + + # now that `message_box` has been called, wait for an arbitrary time period + # in hopes that if there is an exception, it will have been already raised + # and exited the process + time.sleep(2) + + # if the process is alive, assume the message box was successfully created + self.assertTrue(proc.is_alive()) + # make sure the process is terminated in case it is still alive + proc.terminate() + class MessageBoxInteractiveTest(unittest.TestCase): __tags__ = ["interactive"] From 78cdf2c9492d972c46dd42016319ba8acb9dd555 Mon Sep 17 00:00:00 2001 From: Matiiss <83066658+Matiiss@users.noreply.github.com> Date: Fri, 23 Feb 2024 01:38:03 +0200 Subject: [PATCH 2/4] change the test to an interactive one because multiprocessing is failing on CI --- test/display_test.py | 34 +++------------------------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/test/display_test.py b/test/display_test.py index ecf9a361df..b3abc9583f 100644 --- a/test/display_test.py +++ b/test/display_test.py @@ -951,37 +951,6 @@ def test_messagebox_args(self): # the important bit is `parent_window=None` self.assertRaises(TypeError, lambda: mb("", buttons=(), parent_window=None)) - @staticmethod - def _create_message_box_in_process(has_reached_message_box_flag): - has_reached_message_box_flag.value = 1 - pygame.display.message_box("", parent_window=None) - - @unittest.skipIf( - sys.platform.startswith("emscripten"), "multiprocessing not available" - ) - def test_message_box_with_parent_window_none(self): - import multiprocessing - - has_reached_message_box = multiprocessing.Value("i", 0) - proc = multiprocessing.Process( - target=self._create_message_box_in_process, args=(has_reached_message_box,) - ) - proc.start() - - # wait until the process actually reaches the line where it calls `message_box` - while not has_reached_message_box.value: - pass - - # now that `message_box` has been called, wait for an arbitrary time period - # in hopes that if there is an exception, it will have been already raised - # and exited the process - time.sleep(2) - - # if the process is alive, assume the message box was successfully created - self.assertTrue(proc.is_alive()) - # make sure the process is terminated in case it is still alive - proc.terminate() - class MessageBoxInteractiveTest(unittest.TestCase): __tags__ = ["interactive"] @@ -1048,6 +1017,9 @@ def test_message_box_buttons(self): buttons=("Yes", "No"), ) self.assertEqual(result, 0) + + def test_message_box_parent_window_none(self): + pygame.display.message_box("Test", "Just close this messagebox", parent_window=None) if __name__ == "__main__": From 950ec1393084e1fcb9d7e0682b918d05625de859 Mon Sep 17 00:00:00 2001 From: Matiiss <83066658+Matiiss@users.noreply.github.com> Date: Fri, 23 Feb 2024 01:39:10 +0200 Subject: [PATCH 3/4] formatting --- test/display_test.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/display_test.py b/test/display_test.py index b3abc9583f..3711bdc977 100644 --- a/test/display_test.py +++ b/test/display_test.py @@ -1017,9 +1017,11 @@ def test_message_box_buttons(self): buttons=("Yes", "No"), ) self.assertEqual(result, 0) - + def test_message_box_parent_window_none(self): - pygame.display.message_box("Test", "Just close this messagebox", parent_window=None) + pygame.display.message_box( + "Test", "Just close this messagebox", parent_window=None + ) if __name__ == "__main__": From 40af2bbaf87cb3f1f64f2de63084080670170c6a Mon Sep 17 00:00:00 2001 From: Matiiss <83066658+Matiiss@users.noreply.github.com> Date: Sat, 8 Jun 2024 02:16:30 +0300 Subject: [PATCH 4/4] Removed sketchy test --- test/display_test.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/display_test.py b/test/display_test.py index 4d75530924..0ffabccfba 100644 --- a/test/display_test.py +++ b/test/display_test.py @@ -955,9 +955,6 @@ def test_messagebox_args(self): self.assertRaises(TypeError, lambda: mb("", buttons=())) self.assertRaises(TypeError, lambda: mb("", parent_window=123456)) - # the important bit is `parent_window=None` - self.assertRaises(TypeError, lambda: mb("", buttons=(), parent_window=None)) - class MessageBoxInteractiveTest(unittest.TestCase): __tags__ = ["interactive"]