search

py-sdl / py-sdl2

Python ctypes wrapper around SDL2
Other
304 stars 49 forks source link

tests: Don't assume SDL_Init will leave SDL_GetError unset #258

Closed smcv closed 1 year ago

smcv commented 1 year ago

PR Description

SDL_GetError works like Standard C errno: if a call fails it will (generally) set the error indicator, but if a call succeeds there is no guarantee about whether the error indicator has been set, cleared, or left at its previous value.

The SDL documentation says this:

The message is only applicable when an SDL function has signaled an
error. You must check the return values of SDL function calls to
determine when to appropriately call SDL_GetError(). You should not
use the results of SDL_GetError() to decide if an error has occurred!
Sometimes SDL will set an error string even when reporting success.

In particular, with a SDL 2.27.x git snapshot, a successful SDL_Init can leave the error indicator set to a message about inability to open an input device node, and this does not mean it has failed.

In this commit I have not attempted to check or fix other uses of SDL_GetError, only the ones that follow SDL_Init. This is enough to make the test suite pass on Debian 12 alpha with SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy SDL_RENDER_DRIVER=software for now, but the other SDL_GetError() calls in the test suite should ideally also be checked.

Partial solution for: https://github.com/py-sdl/py-sdl2/issues/257

Merge Checklist

a-hurst commented 1 year ago

Ah yep, I learned about this convention unexpectedly a release or two ago. I've already fixed a few older tests that were failing due to non-fatal errors, but clearly I wasn't very thorough. Thanks for the heads-up about 2.27 as well as the patch!

When things calm down a bit with work and my studies I'll make time to comprehensively find/fix this assumption throughout the test suite so I can be robust against future releases.