a-hurst
commented
1 year ago Thanks again, I'll try combing through the other test modules and fixing the other spots I missed!
I also like the convention of documenting the checks that aren't 100% safe for the functions that return void, I'll try to keep that up elsewhere too.
PR Description
This brings py-sdl2 a bit closer to fully resolving #257, by reducing the number of places where it assumes
SDL_GetError != ''implies that there was an error. #267, #269, etc. are targeted versions of this addressing places where a newer SDL2 release does break that assumption, whereas this PR is a preventative version addressing places where a newer SDL2 release might break that assumption but has not done so yet.This PR only covers
video_test, treating all other tests as out-of-scope. Many other test modules will eventually need a change similar to this one.video_test: Don't check error unless a function failed
Helps: https://github.com/py-sdl/py-sdl2/issues/257
video_test: Use _check_error_msg() a bit more often
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
Merge Checklist
closes #<issue-number>to automatically close an issue