diff --git a/fix-partially-resolve-video_test.patch b/fix-partially-resolve-video_test.patch new file mode 100644 index 0000000..7303aea --- /dev/null +++ b/fix-partially-resolve-video_test.patch @@ -0,0 +1,240 @@ +From ed28ea6305dede1b6ba046e36ddae9ba2016b62e Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Sat, 19 Aug 2023 23:44:03 +0100 +Subject: [PATCH] Partially resolve #257: video_test (#270) + +* video_test: Don't check error unless a function failed + +Helps: https://github.com/py-sdl/py-sdl2/issues/257 +Signed-off-by: Simon McVittie + +* video_test: Use _check_error_msg() a bit more often + +Signed-off-by: Simon McVittie + +* video_test: Mitigate unsuitability of SDL_GetError() for detecting failure + +SDL_GetError() is like errno: it's documented not to be suitable for +detecting failure, only for getting more details if failure was already +detected (its result is unspecified on success, because a successful +API call might have been implemented by doing something that failed, +detecting that, and falling back to doing something different). +However, some functions in SDL2 return void, so we have no other way +to tell whether they have failed (they do return a result in SDL3). + +To make it less likely that upgrading SDL2 will make these tests regress, +clear the error indicator immediately before calling the function under +test. It is still not guaranteed to be empty on success, but at least +this way we make sure it doesn't already contain an error message from +a previous function call. + +Helps: https://github.com/py-sdl/py-sdl2/issues/257 +Signed-off-by: Simon McVittie + +--------- + +Signed-off-by: Simon McVittie +--- + sdl2/test/video_test.py | 58 +++++++++++++++++++++-------------------- + 1 file changed, 30 insertions(+), 28 deletions(-) + +diff --git a/sdl2/test/video_test.py b/sdl2/test/video_test.py +index 83cb620..ca5a8e4 100644 +--- a/sdl2/test/video_test.py ++++ b/sdl2/test/video_test.py +@@ -52,8 +52,7 @@ def with_sdl_gl(with_sdl): + def window(with_sdl): + flag = sdl2.SDL_WINDOW_BORDERLESS + w = sdl2.SDL_CreateWindow(b"Test", 10, 40, 12, 13, flag) +- if not w: +- assert SDL_GetError() == b"" ++ assert w, _check_error_msg() + assert isinstance(w.contents, sdl2.SDL_Window) + sdl2.SDL_ClearError() + yield w +@@ -63,8 +62,7 @@ def window(with_sdl): + def decorated_window(with_sdl): + flag = sdl2.SDL_WINDOW_RESIZABLE + w = sdl2.SDL_CreateWindow(b"Test", 10, 40, 12, 13, flag) +- if not w: +- assert SDL_GetError() == b"" ++ assert w, _check_error_msg() + assert isinstance(w.contents, sdl2.SDL_Window) + sdl2.SDL_ClearError() + yield w +@@ -74,20 +72,18 @@ def decorated_window(with_sdl): + def gl_window(with_sdl_gl): + flag = sdl2.SDL_WINDOW_OPENGL + w = sdl2.SDL_CreateWindow(b"OpenGL", 10, 40, 12, 13, flag) +- if not w: +- assert SDL_GetError() == b"" ++ assert w, _check_error_msg() + assert isinstance(w.contents, sdl2.SDL_Window) + sdl2.SDL_ClearError() + ctx = sdl2.SDL_GL_CreateContext(w) +- assert SDL_GetError() == b"" ++ assert ctx, _check_error_msg() + yield (w, ctx) + sdl2.SDL_GL_DeleteContext(ctx) + sdl2.SDL_DestroyWindow(w) + + def _create_window(name, h, w, x, y, flags): + window = sdl2.SDL_CreateWindow(name, h, w, x, y, flags) +- if not window: +- assert SDL_GetError() == b"" ++ assert window, _check_error_msg() + assert isinstance(window.contents, sdl2.SDL_Window) + sdl2.SDL_ClearError() + return window +@@ -193,7 +189,7 @@ def test_SDL_VideoInitQuit(): + # Test with default driver + assert sdl2.SDL_WasInit(0) & sdl2.SDL_INIT_VIDEO != sdl2.SDL_INIT_VIDEO + ret = sdl2.SDL_VideoInit(None) +- assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace') ++ assert ret == 0, _check_error_msg() + assert sdl2.SDL_GetCurrentVideoDriver() # If initialized, should be string + sdl2.SDL_VideoQuit() + assert not sdl2.SDL_GetCurrentVideoDriver() +@@ -234,8 +230,7 @@ def test_SDL_GetDisplayMode(with_sdl): + for mode in range(modes): + dmode = sdl2.SDL_DisplayMode() + ret = sdl2.SDL_GetDisplayMode(index, mode, byref(dmode)) +- assert sdl2.SDL_GetError() == b"" +- assert ret == 0 ++ assert ret == 0, _check_error_msg() + if not DRIVER_DUMMY: + assert dmode.w > 0 + assert dmode.h > 0 +@@ -245,8 +240,7 @@ def test_SDL_GetCurrentDisplayMode(with_sdl): + for index in range(numdisplays): + dmode = sdl2.SDL_DisplayMode() + ret = sdl2.SDL_GetCurrentDisplayMode(index, byref(dmode)) +- assert sdl2.SDL_GetError() == b"" +- assert ret == 0 ++ assert ret == 0, _check_error_msg() + assert dmode.w > 0 + assert dmode.h > 0 + +@@ -255,8 +249,7 @@ def test_SDL_GetDesktopDisplayMode(with_sdl): + for index in range(numdisplays): + dmode = sdl2.SDL_DisplayMode() + ret = sdl2.SDL_GetDesktopDisplayMode(index, byref(dmode)) +- assert sdl2.SDL_GetError() == b"" +- assert ret == 0 ++ assert ret == 0, _check_error_msg() + assert dmode.w > 0 + assert dmode.h > 0 + +@@ -266,8 +259,7 @@ def test_SDL_GetClosestDisplayMode(with_sdl): + for index in range(numdisplays): + dmode = sdl2.SDL_DisplayMode() + ret = sdl2.SDL_GetCurrentDisplayMode(index, byref(dmode)) +- assert sdl2.SDL_GetError() == b"" +- assert ret == 0 ++ assert ret == 0, _check_error_msg() + cmode = sdl2.SDL_DisplayMode( + dmode.format, dmode.w - 1, dmode.h - 1, dmode.refresh_rate + ) +@@ -286,8 +278,7 @@ def test_SDL_GetDisplayBounds(with_sdl): + for index in range(numdisplays): + bounds = rect.SDL_Rect() + ret = sdl2.SDL_GetDisplayBounds(index, byref(bounds)) +- assert sdl2.SDL_GetError() == b"" +- assert ret == 0 ++ assert ret == 0, _check_error_msg() + assert bounds.w > 0 + assert bounds.h > 0 + assert not rect.SDL_RectEmpty(bounds) +@@ -345,8 +336,8 @@ def test_GetDisplayInfo(with_sdl): + def test_SDL_CreateDestroyWindow(with_sdl): + flag = sdl2.SDL_WINDOW_BORDERLESS + window = sdl2.SDL_CreateWindow(b"Test", 10, 40, 12, 13, flag) ++ assert window, _check_error_msg() + if not isinstance(window.contents, sdl2.SDL_Window): +- assert SDL_GetError() == b"" + assert isinstance(window.contents, sdl2.SDL_Window) + sdl2.SDL_DestroyWindow(window) + +@@ -453,7 +444,10 @@ def test_SDL_SetWindowIcon(window): + 0, 16, 16, 16, 0xF000, 0x0F00, 0x00F0, 0x000F + ) + assert isinstance(sf.contents, surface.SDL_Surface) ++ sdl2.SDL_ClearError() + sdl2.SDL_SetWindowIcon(window, sf) ++ # TODO: This is not 100% safe, but in SDL2, SetWindowIcon returns void, ++ # so we can't reliably detect error + assert SDL_GetError() == b"" + + @pytest.mark.xfail(is_pypy, reason="PyPy can't create proper py_object values") +@@ -535,7 +529,10 @@ def test_SDL_GetSetWindowMinimumSize(window): + sdl2.SDL_GetWindowSize(window, byref(sx), byref(sy)) + assert (sx.value, sy.value) == (12, 13) + # Set and verify the minimum window size ++ sdl2.SDL_ClearError() + sdl2.SDL_SetWindowMinimumSize(window, 10, 10) ++ # TODO: This is not 100% safe, but in SDL2, SetWindowMinimumSize returns ++ # void, so we can't reliably detect error + assert SDL_GetError() == b"" + sdl2.SDL_GetWindowMinimumSize(window, byref(minx), byref(miny)) + assert (minx.value, miny.value) == (10, 10) +@@ -549,8 +546,11 @@ def test_SDL_GetSetWindowMaximumSize(window): + maxx, maxy = c_int(0), c_int(0) + sdl2.SDL_GetWindowSize(window, byref(sx), byref(sy)) + assert (sx.value, sy.value) == (12, 13) ++ sdl2.SDL_ClearError() + # Set and verify the maximum window size + sdl2.SDL_SetWindowMaximumSize(window, 32, 32) ++ # TODO: This is not 100% safe, but in SDL2, SetWindowMaximumSize returns ++ # void, so we can't reliably detect error + assert SDL_GetError() == b"" + sdl2.SDL_GetWindowMaximumSize(window, byref(maxx), byref(maxy)) + assert (maxx.value, maxy.value) == (32, 32) +@@ -660,7 +660,7 @@ def test_SDL_HasWindowSurface(window): + + def test_SDL_GetWindowSurface(window): + sf = sdl2.SDL_GetWindowSurface(window) +- assert SDL_GetError() == b"" ++ assert sf, _check_error_msg() + assert isinstance(sf.contents, surface.SDL_Surface) + + def test_SDL_UpdateWindowSurface(window): +@@ -857,23 +857,22 @@ def test_SDL_GL_CreateDeleteContext(with_sdl_gl): + b"OpenGL", 10, 40, 32, 24, sdl2.SDL_WINDOW_OPENGL + ) + ctx = sdl2.SDL_GL_CreateContext(window) +- assert SDL_GetError() == b"" ++ assert ctx, _check_error_msg() + sdl2.SDL_GL_DeleteContext(ctx) + ctx = sdl2.SDL_GL_CreateContext(window) +- assert SDL_GetError() == b"" ++ assert ctx, _check_error_msg() + sdl2.SDL_GL_DeleteContext(ctx) + sdl2.SDL_DestroyWindow(window) + + @pytest.mark.skipif(DRIVER_DUMMY, reason="Doesn't work with dummy driver") + def test_SDL_GL_GetProcAddress(gl_window): + procaddr = sdl2.SDL_GL_GetProcAddress(b"glGetString") +- assert SDL_GetError() == b"" +- assert procaddr is not None and int(procaddr) != 0 ++ assert procaddr is not None and int(procaddr) != 0, _check_error_msg() + + @pytest.mark.skipif(DRIVER_DUMMY, reason="Doesn't work with dummy driver") + def test_SDL_GL_ExtensionSupported(gl_window): +- assert sdl2.SDL_GL_ExtensionSupported(b"GL_EXT_bgra") +- assert SDL_GetError() == b"" ++ sdl2.SDL_ClearError() ++ assert sdl2.SDL_GL_ExtensionSupported(b"GL_EXT_bgra"), _check_error_msg() + + @pytest.mark.skipif(DRIVER_DUMMY, reason="Doesn't work with dummy driver") + def test_SDL_GL_GetSetResetAttribute(with_sdl_gl): +@@ -946,7 +945,10 @@ def test_SDL_GL_SwapWindow(gl_window): + window, ctx = gl_window + ret = sdl2.SDL_GL_MakeCurrent(window, ctx) + assert ret == 0, _check_error_msg() ++ sdl2.SDL_ClearError() + sdl2.SDL_GL_SwapWindow(window) + sdl2.SDL_GL_SwapWindow(window) + sdl2.SDL_GL_SwapWindow(window) ++ # TODO: This is not 100% safe, but in SDL2, GL_SwapWindow returns ++ # void, so we can't reliably detect error + assert SDL_GetError() == b"" diff --git a/fix-test-SDL_hid_enumerate.patch b/fix-test-SDL_hid_enumerate.patch new file mode 100644 index 0000000..7ac41d1 --- /dev/null +++ b/fix-test-SDL_hid_enumerate.patch @@ -0,0 +1,43 @@ +From d7c0604381f6cbefa2c8b51c84b879e2f927e91e Mon Sep 17 00:00:00 2001 +From: Simon McVittie +Date: Sat, 19 Aug 2023 23:38:36 +0100 +Subject: [PATCH] test: Don't assert that SDL_hid_enumerate doesn't set error + (#269) + +On my Linux system, enumeration succeeds, but the error indicator gets +set as a side-effect (which appears to be because the loader checks +whether the symbol exists in SDL or a direct dependency before it +dlopens libudev). + +The API of SDL_hid_enumerate does not make it possible to distinguish +between successfully returning an empty list of devices (returns NULL +with the error indicator in an undefined state) and a failure (returns +NULL with the error indicator set), and systems that run automated tests +usually don't have any HID game controllers connected, so we can't make +any meaningful use of the error indicator here. + +Helps: https://github.com/py-sdl/py-sdl2/issues/257 + +Signed-off-by: Simon McVittie +--- + sdl2/test/hidapi_test.py | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/sdl2/test/hidapi_test.py b/sdl2/test/hidapi_test.py +index 68bb9f9..5422d42 100644 +--- a/sdl2/test/hidapi_test.py ++++ b/sdl2/test/hidapi_test.py +@@ -38,10 +38,11 @@ def test_SDL_hid_device_change_count(hidapi_setup): + + def test_SDL_hid_enumerate(hidapi_setup): + devices = sdl2.SDL_hid_enumerate(0, 0) +- assert SDL_GetError() == b"" ++ # Cannot check the error indicator here: a non-error empty list is ++ # indistinguishable from an error, and it is not guaranteed that the ++ # error indicator will not be set as a side-effect of a successful load + if devices != None: + sdl2.SDL_hid_free_enumeration(devices) +- assert SDL_GetError() == b"" + + + @pytest.mark.skip("not implemented") diff --git a/fix-tests-SDL_GetError.patch b/fix-tests-SDL_GetError.patch new file mode 100644 index 0000000..5b748d3 --- /dev/null +++ b/fix-tests-SDL_GetError.patch @@ -0,0 +1,148 @@ +From 1c865e3f751e678f3ad2d8f3fca17a0755fbeaf7 Mon Sep 17 00:00:00 2001 +From: basak +Date: Fri, 11 Aug 2023 17:52:01 +0100 +Subject: [PATCH] tests: SDL_GetError() != b'' isn't an error (#267) + +* tests: SDL_GetError() != b'' isn't an error + +In Ubuntu, we're seeing test failures of the following pattern when +moving from libsdl2 2.28.1+dfsg-1 to 2.28.2+dfsg-1: + + > assert sdl2.SDL_GetError() == b"" + E AssertionError: assert b'Unexpected ...r element crc' == b'' + E Full diff: + E - b'' + E + b'Unexpected controller element crc' + +This looks like an issue similar to that fixed in commit 8c39f40. We +should check the relevant return value, and only if it indicates failure +should we attach any particular meaning to the return value of +SDL_GetError(). + +* Add myself to AUTHORS.txt + +As instructed in #267 +--- + AUTHORS.txt | 1 + + sdl2/test/gamecontroller_test.py | 16 +++------------- + 2 files changed, 4 insertions(+), 13 deletions(-) + +diff --git a/AUTHORS.txt b/AUTHORS.txt +index 287109e..6d40500 100644 +--- a/AUTHORS.txt ++++ b/AUTHORS.txt +@@ -45,6 +45,7 @@ Thanks to everyone else for their assistance, support, fixes and improvements: + * Radomir Dopieralski + * Rob Rolls + * Robert Winkler ++* Robie Basak + * Roger Flores + * Simon McVittie + * smea lum +diff --git a/sdl2/test/gamecontroller_test.py b/sdl2/test/gamecontroller_test.py +index 7a00aac..f509a88 100644 +--- a/sdl2/test/gamecontroller_test.py ++++ b/sdl2/test/gamecontroller_test.py +@@ -51,7 +51,7 @@ def gamepads(with_sdl): + for i in range(count): + if sdl2.SDL_IsGameController(i) == SDL_TRUE: + pad = sdl2.SDL_GameControllerOpen(i) +- assert sdl2.SDL_GetError() == b"" ++ assert pad, _check_error_msg() + assert isinstance(pad.contents, sdl2.SDL_GameController) + devices.append(pad) + yield devices +@@ -133,21 +133,18 @@ def test_SDL_GameControllerMappingForGUID(with_sdl): + def test_SDL_GameControllerMapping(gamepads): + for pad in gamepads: + mapping = sdl2.SDL_GameControllerMapping(pad) +- assert SDL_GetError() == b"" + assert mapping == None or type(mapping) in (str, bytes) + + def test_SDL_IsGameController(with_sdl): + count = joystick.SDL_NumJoysticks() + for index in range(count): + ret = sdl2.SDL_IsGameController(index) +- assert sdl2.SDL_GetError() == b"" + assert ret in [SDL_TRUE, SDL_FALSE] + + def test_SDL_GameControllerNameForIndex(with_sdl): + count = joystick.SDL_NumJoysticks() + for index in range(count): + name = sdl2.SDL_GameControllerNameForIndex(index) +- assert sdl2.SDL_GetError() == b"" + assert name == None or type(name) in (str, bytes) + + @pytest.mark.skipif(sdl2.dll.version < 2231, reason="not available") +@@ -174,7 +171,7 @@ def test_SDL_GameControllerOpenClose(with_sdl): + for index in range(count): + if sdl2.SDL_IsGameController(index) == SDL_TRUE: + pad = sdl2.SDL_GameControllerOpen(index) +- assert sdl2.SDL_GetError() == b"" ++ assert pad, _check_error_msg() + assert isinstance(pad.contents, sdl2.SDL_GameController) + sdl2.SDL_GameControllerClose(pad) + +@@ -220,7 +217,6 @@ def test_SDL_GameControllerPath(gamepads): + def test_SDL_GameControllerGetType(gamepads): + for pad in gamepads: + padtype = sdl2.SDL_GameControllerGetType(pad) +- assert SDL_GetError() == b"" + assert padtype in gamepad_types + if is_virtual(pad): + assert padtype == sdl2.SDL_CONTROLLER_TYPE_VIRTUAL +@@ -244,7 +240,6 @@ def test_SDL_GameControllerSetPlayerIndex(gamepads): + def test_SDL_GameControllerGetVendor(gamepads): + for pad in gamepads: + vid = sdl2.SDL_GameControllerGetVendor(pad) +- assert SDL_GetError() == b"" + if not is_virtual(pad): + assert vid > 0 + +@@ -252,7 +247,6 @@ def test_SDL_GameControllerGetVendor(gamepads): + def test_SDL_GameControllerGetProduct(gamepads): + for pad in gamepads: + pid = sdl2.SDL_GameControllerGetProduct(pad) +- assert SDL_GetError() == b"" + if not is_virtual(pad): + assert pid > 0 + +@@ -260,21 +254,18 @@ def test_SDL_GameControllerGetProduct(gamepads): + def test_SDL_GameControllerGetProductVersion(gamepads): + for pad in gamepads: + pver = sdl2.SDL_GameControllerGetProductVersion(pad) +- assert SDL_GetError() == b"" + assert pver >= 0 + + @pytest.mark.skipif(sdl2.dll.version < 2231, reason="not available") + def test_SDL_GameControllerGetFirmwareVersion(gamepads): + for pad in gamepads: + fw_ver = sdl2.SDL_GameControllerGetFirmwareVersion(pad) +- assert SDL_GetError() == b"" + assert fw_ver >= 0 + + @pytest.mark.skipif(sdl2.dll.version < 2014, reason="not available") + def test_SDL_GameControllerGetSerial(gamepads): + for pad in gamepads: + serial = sdl2.SDL_GameControllerGetSerial(pad) +- assert SDL_GetError() == b"" + assert serial == None or type(serial) in (str, bytes) + + def test_SDL_GameControllerGetAttached(gamepads): +@@ -285,7 +276,7 @@ def test_SDL_GameControllerGetAttached(gamepads): + def test_SDL_GameControllerGetJoystick(gamepads): + for pad in gamepads: + stick = sdl2.SDL_GameControllerGetJoystick(pad) +- assert SDL_GetError() == b"" ++ assert stick, _check_error_msg() + assert isinstance(stick.contents, joystick.SDL_Joystick) + + def test_SDL_GameControllerEventState(with_sdl): +@@ -298,7 +289,6 @@ def test_SDL_GameControllerEventState(with_sdl): + def test_SDL_GameControllerUpdate(with_sdl): + # NOTE: Returns void, can't really test anything else + sdl2.SDL_GameControllerUpdate() +- assert SDL_GetError() == b"" + + def test_SDL_GameControllerGetAxisFromString(with_sdl): + expected = { diff --git a/fix-tests.patch b/fix-tests.patch deleted file mode 100644 index 7523a3a..0000000 --- a/fix-tests.patch +++ /dev/null @@ -1,17 +0,0 @@ ---- a/sdl2/test/hidapi_test.py -+++ b/sdl2/test/hidapi_test.py -@@ -38,10 +38,12 @@ def test_SDL_hid_device_change_count(hid - - def test_SDL_hid_enumerate(hidapi_setup): - devices = sdl2.SDL_hid_enumerate(0, 0) -- assert SDL_GetError() == b"" -+ # Ignore errors like Unable to open /dev/input/... -+ #assert SDL_GetError() == b"" - if devices != None: - sdl2.SDL_hid_free_enumeration(devices) -- assert SDL_GetError() == b"" -+ # Ignore errors like Unable to open /dev/input/... -+ #assert SDL_GetError() == b"" - - - @pytest.mark.skip("not implemented") diff --git a/python-PySDL2.changes b/python-PySDL2.changes index cfd53ad..12ea4fd 100644 --- a/python-PySDL2.changes +++ b/python-PySDL2.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Sun Aug 20 11:55:43 UTC 2023 - munix9@googlemail.com + +- Remove fix-tests.patch in favor of upstream patches +- Add upstream patches to resolve test failures: + fix-partially-resolve-video_test.patch + fix-test-SDL_hid_enumerate.patch + fix-tests-SDL_GetError.patch + ------------------------------------------------------------------- Sun Jul 16 06:08:53 UTC 2023 - munix9@googlemail.com diff --git a/python-PySDL2.spec b/python-PySDL2.spec index 10f0394..19abbab 100644 --- a/python-PySDL2.spec +++ b/python-PySDL2.spec @@ -24,8 +24,12 @@ Summary: Python ctypes wrapper around SDL2 License: SUSE-Public-Domain URL: https://github.com/py-sdl/py-sdl2 Source: https://files.pythonhosted.org/packages/source/P/PySDL2/PySDL2-%{version}.tar.gz -# PATCH-FIX-OPENSUSE fix-tests.patch to make test work in chroot env without access to /dev/input -Patch0: fix-tests.patch +# PATCH-FIX-UPSTREAM fix-tests-SDL_GetError.patch -- based on commit 1c865e3 +Patch0: https://github.com/py-sdl/py-sdl2/commit/1c865e3f751e678f3ad2d8f3fca17a0755fbeaf7.patch#/fix-tests-SDL_GetError.patch +# PATCH-FIX-UPSTREAM fix-test-SDL_hid_enumerate.patch -- based on commit d7c0604 +Patch1: https://github.com/py-sdl/py-sdl2/commit/d7c0604381f6cbefa2c8b51c84b879e2f927e91e.patch#/fix-test-SDL_hid_enumerate.patch +# PATCH-FIX-UPSTREAM fix-partially-resolve-video_test.patch -- based on commit ed28ea6 +Patch2: https://github.com/py-sdl/py-sdl2/commit/ed28ea6305dede1b6ba046e36ddae9ba2016b62e.patch#/fix-partially-resolve-video_test.patch BuildRequires: %{python_module pip} BuildRequires: %{python_module setuptools} BuildRequires: %{python_module wheel}