Skip to content

Commit

Permalink
tests: Don't assume SDL_Init will leave SDL_GetError unset (#258)
Browse files Browse the repository at this point in the history
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: #257

Signed-off-by: Simon McVittie <smcv@collabora.com>
  • Loading branch information
smcv committed Feb 12, 2023
1 parent 282a1df commit 8c39f40
Show file tree
Hide file tree
Showing 10 changed files with 15 additions and 24 deletions.
5 changes: 2 additions & 3 deletions sdl2/test/audio_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ def with_sdl_audio():
sdl2.SDL_Quit()
sdl2.SDL_ClearError()
ret = sdl2.SDL_Init(sdl2.SDL_INIT_VIDEO | sdl2.SDL_INIT_AUDIO)
assert sdl2.SDL_GetError() == b""
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
yield
sdl2.SDL_Quit()
# Reset original audio driver in environment
Expand Down Expand Up @@ -308,7 +307,7 @@ def test_SDL_GetDefaultAudioInfo(with_default_driver):
# If method isn't implemented for the current back end, just skip
if ret < 0 and b"not supported" in sdl2.SDL_GetError():
pytest.skip("not supported by driver")
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
# Validate frequency and channel count were set
hz = outspec.freq
fmt = FORMAT_NAME_MAP[outspec.format] if outspec.format > 0 else 'unknown'
Expand Down
3 changes: 1 addition & 2 deletions sdl2/test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
def with_sdl():
sdl2.SDL_ClearError()
ret = sdl2.SDL_Init(sdl2.SDL_INIT_VIDEO | sdl2.SDL_INIT_TIMER)
assert sdl2.SDL_GetError() == b""
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
yield
sdl2.SDL_Quit()

Expand Down
5 changes: 2 additions & 3 deletions sdl2/test/gamecontroller_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# Get status of gamepad support/availability before running tests
SDL_ClearError()
ret = SDL_Init(sdl2.SDL_INIT_GAMECONTROLLER)
gamepad_works = ret == 0 and SDL_GetError() == b""
gamepad_works = ret == 0
SDL_Quit()

pytestmark = pytest.mark.skipif(not gamepad_works, reason="system unsupported")
Expand All @@ -35,8 +35,7 @@ def with_sdl():
sdl2.SDL_ClearError()
sdl2.SDL_SetHint(b"SDL_JOYSTICK_ALLOW_BACKGROUND_EVENTS", b"1")
ret = sdl2.SDL_Init(sdl2.SDL_INIT_VIDEO | sdl2.SDL_INIT_GAMECONTROLLER)
assert sdl2.SDL_GetError() == b""
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
# Also initialize a virtual joystick (if supported)
if sdl2.dll.version >= 2014:
virt_type = joystick.SDL_JOYSTICK_TYPE_GAMECONTROLLER
Expand Down
3 changes: 1 addition & 2 deletions sdl2/test/hints_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
def with_sdl():
sdl2.SDL_ClearError()
ret = sdl2.SDL_Init(sdl2.SDL_INIT_VIDEO)
assert sdl2.SDL_GetError() == b""
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
yield
sdl2.SDL_Quit()

Expand Down
5 changes: 2 additions & 3 deletions sdl2/test/joystick_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
any_joysticks = False
SDL_ClearError()
ret = SDL_Init(SDL_INIT_JOYSTICK)
joystick_works = ret == 0 and SDL_GetError() == b""
joystick_works = ret == 0
if joystick_works:
devices = sdl2.SDL_NumJoysticks()
if sdl2.dll.version >= 2014:
Expand Down Expand Up @@ -42,8 +42,7 @@ def with_sdl():
sdl2.SDL_ClearError()
sdl2.SDL_SetHint(b"SDL_JOYSTICK_ALLOW_BACKGROUND_EVENTS", b"1")
ret = sdl2.SDL_Init(sdl2.SDL_INIT_VIDEO | sdl2.SDL_INIT_JOYSTICK)
assert sdl2.SDL_GetError() == b""
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
# Also initialize a virtual joystick (if supported)
if sdl2.dll.version >= 2014:
virt_type = sdl2.SDL_JOYSTICK_TYPE_GAMECONTROLLER
Expand Down
7 changes: 3 additions & 4 deletions sdl2/test/sdl_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def test_SDL_Init():
ret = sdl2.SDL_Init(flags)
err = sdl2.SDL_GetError()
if name in ['timer', 'audio', 'video', 'events']:
assert ret == 0
assert ret == 0, err.decode('utf-8', 'replace')
if err:
err = err.decode('utf-8')
print("Error loading {0} subsystem: {1}".format(name, err))
Expand All @@ -40,12 +40,11 @@ def test_SDL_Init():
def test_SDL_InitSubSystem():
sdl2.SDL_ClearError()
ret = sdl2.SDL_Init(SDL_INIT_VIDEO | SDL_INIT_AUDIO)
assert sdl2.SDL_GetError() == b""
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
# Test initializing an additional subsystem
assert sdl2.SDL_WasInit(0) & SDL_INIT_TIMER != SDL_INIT_TIMER
ret = sdl2.SDL_InitSubSystem(SDL_INIT_TIMER)
assert sdl2.SDL_GetError() == b""
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
assert sdl2.SDL_WasInit(0) & SDL_INIT_TIMER == SDL_INIT_TIMER
# Test shutting down a single subsystem
sdl2.SDL_QuitSubSystem(SDL_INIT_AUDIO)
Expand Down
3 changes: 1 addition & 2 deletions sdl2/test/sdlmixer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ def with_sdl_mixer():
# Initialize SDL2 with video and audio subsystems
sdl2.SDL_ClearError()
ret = sdl2.SDL_Init(sdl2.SDL_INIT_VIDEO | sdl2.SDL_INIT_AUDIO)
assert sdl2.SDL_GetError() == b""
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
# Initialize SDL_mixer and open an audio device
flags = (
sdlmixer.MIX_INIT_FLAC | sdlmixer.MIX_INIT_MOD | sdlmixer.MIX_INIT_MP3 |
Expand Down
2 changes: 1 addition & 1 deletion sdl2/test/sensor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

def test_SDL_NumSensors():
ret = SDL_Init(SDL_INIT_SENSOR)
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
retval = sdl2.SDL_NumSensors()
SDL_Quit()
assert retval >= 0
Expand Down
3 changes: 1 addition & 2 deletions sdl2/test/touch_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ def with_sdl():
sdl2.SDL_SetHint(sdl2.SDL_HINT_MOUSE_TOUCH_EVENTS, b"1")
sdl2.SDL_ClearError()
ret = sdl2.SDL_Init(sdl2.SDL_INIT_VIDEO)
assert sdl2.SDL_GetError() == b""
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
yield
sdl2.SDL_Quit()

Expand Down
3 changes: 1 addition & 2 deletions sdl2/test/video_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,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 sdl2.SDL_GetError() == b""
assert ret == 0
assert ret == 0, sdl2.SDL_GetError().decode('utf-8', 'replace')
assert sdl2.SDL_GetCurrentVideoDriver() # If initialized, should be string
sdl2.SDL_VideoQuit()
assert not sdl2.SDL_GetCurrentVideoDriver()
Expand Down

0 comments on commit 8c39f40

Please sign in to comment.