py-sdl / py-sdl2

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

ValueError: NULL pointer access while running the test suite #241

Closed mcepl closed 2 years ago

mcepl commented 2 years ago

When packaging the package (from tarball 0.9.13 from PyPI) I get these errors when running the test suite:

[   40s] ==================================== ERRORS ====================================
[   40s] ___________________ ERROR at setup of test_Mix_GetMusicType ____________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     @pytest.fixture
[   40s]     def with_music(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         mus = sdlmixer.Mix_LoadMUS(fpath.encode("utf-8"))
[   40s]         err = sdlmixer.Mix_GetError()
[   40s]         sdlmixer.Mix_ClearError()
[   40s] >       assert isinstance(mus.contents, sdlmixer.Mix_Music)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:58: ValueError
[   40s] ___________________ ERROR at setup of test_Mix_GetMusicTitle ___________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     @pytest.fixture
[   40s]     def with_music(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         mus = sdlmixer.Mix_LoadMUS(fpath.encode("utf-8"))
[   40s]         err = sdlmixer.Mix_GetError()
[   40s]         sdlmixer.Mix_ClearError()
[   40s] >       assert isinstance(mus.contents, sdlmixer.Mix_Music)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:58: ValueError
[   40s] _________________ ERROR at setup of test_Mix_GetMusicTitleTag __________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     @pytest.fixture
[   40s]     def with_music(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         mus = sdlmixer.Mix_LoadMUS(fpath.encode("utf-8"))
[   40s]         err = sdlmixer.Mix_GetError()
[   40s]         sdlmixer.Mix_ClearError()
[   40s] >       assert isinstance(mus.contents, sdlmixer.Mix_Music)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:58: ValueError
[   40s] _________________ ERROR at setup of test_Mix_GetMusicArtistTag _________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     @pytest.fixture
[   40s]     def with_music(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         mus = sdlmixer.Mix_LoadMUS(fpath.encode("utf-8"))
[   40s]         err = sdlmixer.Mix_GetError()
[   40s]         sdlmixer.Mix_ClearError()
[   40s] >       assert isinstance(mus.contents, sdlmixer.Mix_Music)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:58: ValueError
[   40s] _________________ ERROR at setup of test_Mix_GetMusicAlbumTag __________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     @pytest.fixture
[   40s]     def with_music(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         mus = sdlmixer.Mix_LoadMUS(fpath.encode("utf-8"))
[   40s]         err = sdlmixer.Mix_GetError()
[   40s]         sdlmixer.Mix_ClearError()
[   40s] >       assert isinstance(mus.contents, sdlmixer.Mix_Music)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:58: ValueError
[   40s] _______________ ERROR at setup of test_Mix_GetMusicCopyrightTag ________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     @pytest.fixture
[   40s]     def with_music(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         mus = sdlmixer.Mix_LoadMUS(fpath.encode("utf-8"))
[   40s]         err = sdlmixer.Mix_GetError()
[   40s]         sdlmixer.Mix_ClearError()
[   40s] >       assert isinstance(mus.contents, sdlmixer.Mix_Music)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:58: ValueError
[   40s] ___________________ ERROR at setup of test_Mix_MusicDuration ___________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     @pytest.fixture
[   40s]     def with_music(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         mus = sdlmixer.Mix_LoadMUS(fpath.encode("utf-8"))
[   40s]         err = sdlmixer.Mix_GetError()
[   40s]         sdlmixer.Mix_ClearError()
[   40s] >       assert isinstance(mus.contents, sdlmixer.Mix_Music)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:58: ValueError
[   40s] =================================== FAILURES ===================================
[   40s] _____________________________ test_Mix_LoadFreeWAV _____________________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     def test_Mix_LoadFreeWAV(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         chk = sdlmixer.Mix_LoadWAV(fpath.encode("utf-8"))
[   40s] >       assert isinstance(chk.contents, sdlmixer.Mix_Chunk)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:145: ValueError
[   40s] _____________________________ test_Mix_LoadWAV_RW ______________________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     def test_Mix_LoadWAV_RW(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         rw = sdl2.SDL_RWFromFile(fpath.encode("utf-8"), b"r")
[   40s]         chk = sdlmixer.Mix_LoadWAV_RW(rw, 1)
[   40s] >       assert isinstance(chk.contents, sdlmixer.Mix_Chunk)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:153: ValueError
[   40s] _____________________________ test_Mix_LoadFreeMUS _____________________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     def test_Mix_LoadFreeMUS(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         mus = sdlmixer.Mix_LoadMUS(fpath.encode("utf-8"))
[   40s] >       assert isinstance(mus.contents, sdlmixer.Mix_Music)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:160: ValueError
[   40s] _____________________________ test_Mix_LoadMUS_RW ______________________________
[   40s] 
[   40s] with_sdl_mixer = None
[   40s] 
[   40s]     def test_Mix_LoadMUS_RW(with_sdl_mixer):
[   40s]         fpath = _get_sound_path("mp3")
[   40s]         rw = sdl2.SDL_RWFromFile(fpath.encode("utf-8"), b"r")
[   40s]         mus = sdlmixer.Mix_LoadMUS_RW(rw, 1)
[   40s] >       assert isinstance(mus.contents, sdlmixer.Mix_Music)
[   40s] E       ValueError: NULL pointer access
[   40s] 
[   40s] sdl2/test/sdlmixer_test.py:168: ValueError
[   40s] =========================== short test summary info ============================

Complete build log with all versions of packages used and steps taken.

Platform (if relevant):

a-hurst commented 2 years ago

Hey @mcepl, it's not 100% clear from the build logs since pytest isn't set to report captured print() output (a few tests print messages describing feature support in the linked SDL2 binaries), but this looks like SDL2_mixer either a) wasn't built with MP3 support, or b) has MP3 support broken for whatever reason.

I can make a patch to skip the MP3-dependent tests if MP3 support is absent in a given Mixer binary, but you may also want to check on openSUSE's current Mixer package to make sure MP3 support isn't unexpectedly broken.

mcepl commented 2 years ago

Hmm, I am really not a specialist on SDL2 (I work on Python packages for SUSE), so I am not sure what to think about this ./configure line of SDL2_mixer:

./configure --host=x86_64-suse-linux-gnu --build=x86_64-suse-linux-gnu
    --program-prefix= --disable-dependency-tracking --prefix=/usr
    --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
    --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
    --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib
    --mandir=/usr/share/man --infodir=/usr/share/info --disable-music-ogg-stb
    --enable-music-ogg-vorbis --disable-music-flac-drflac
    --enable-music-flac-libflac --disable-music-mp3-drmp3 --enable-music-mp3-mpg123
    --disable-music-mod-modplug --enable-music-mod-xmp
    --disable-music-mod-xmp-shared --disable-music-midi-fluidsynth-shared
    --disable-music-ogg-shared --disable-music-flac-shared
    --enable-music-mp3-mpg123-shared --disable-static
a-hurst commented 2 years ago

@mcepl Thanks, that helps a lot! That tells me Mixer is being built with dynamic support for libmpg123 as an MP3 decoder backend, but I can't find a reference to that library in the build log so it's probably a question of Mixer being unable to read MP3 files without that library.

Since I guess it's perfectly valid for a given distro to make mp3 support dynamic/optional, I'll count this as a valid bug in PySDL2's test suite. Here's a patch that should fix this for now, and I'll make sure it's addressed in the next release!

mixer_tests_fix.patch.txt

mcepl commented 2 years ago

Hmm, I don’t know what’s wrong, but it seems your patch didn’t help at all

_log.txt

a-hurst commented 2 years ago

@mcepl Sorry for the delay, been taking some time off! Can you run the tests locally with pytest -vl -rxXP sdl2/test/sdlmixer_test.py and send me the logs? That'll give me a much more informative look at what actually's going wrong with those tests so I can make a proper fix.

mcepl commented 2 years ago

pytest-run-log.txt

Interesting, so it is missing video, not audio device. And yes, I have my serious doubts that we have any video devices accessible in our build machines. Or is it some missing library supporting video?

a-hurst commented 2 years ago

@mcepl Looks like some tests are failing in that last log that were working fine in your earlier ones: can you try re-running with the following environment variables set? These are what I use for headless CI testing:

SDL_VIDEODRIVER=dummy
SDL_AUDIODRIVER=dummy
SDL_RENDER_DRIVER=software

Also, based on the output I was right that MP3 support is missing, so unless I missed something the patch I made should detect that and skip all MP3-requiring tests (the ones that were failing in the original log).

mcepl commented 2 years ago

SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy SDL_RENDER_DRIVER=software

Actually, I had these variables set in my SPEC file (so you should see them in the log from this comment), but when I set in the build environment and run your command again, I got pretty much the same

pytest-run-log.txt

a-hurst commented 2 years ago

Ahhh I see, I wrote my patch wrong: apparently Mix_Init still indicates that MP3 support is loaded correctly even if the required dynamic library isn't available. I rewrote the "check for MP3 support" logic to hopefully be more reliable, let me know if this works!

mixer_test_fix_v2.patch.txt

mcepl commented 2 years ago

Yes, that helped. I don’t need to skip those few tests.

a-hurst commented 2 years ago

Great, so it's fully working now? Or are there any other issues to fix?

mcepl commented 2 years ago

Yes, this problem is gone. We are still skipping plenty of tests. See our SPEC file from line 73 onwards, but I guess it is mostly given some old libs we use or weird architectures you don’t support.

a-hurst commented 2 years ago

@mcepl Glad to hear it! Also, some of those skipped tests (apart from the PowerInfo segfault) look like bugs that have been fixed in the past few PySDL2 releases (e.g. certain tests failing on big-endian, I actually set up Void PPC64 on my old PowerMac G5 to sort that out), so you may be able to remove those lines if you want. There could be bugs I haven't caught yet, though.

mcepl commented 2 years ago

@mcepl Glad to hear it! Also, some of those skipped tests (apart from the PowerInfo segfault) look like bugs that have been fixed in the past few PySDL2 releases (e.g. certain tests failing on big-endian, I actually set up Void PPC64 on my old PowerMac G5 to sort that out), so you may be able to remove those lines if you want. There could be bugs I haven't caught yet, though.

And what do you think? I don’t have to exclude ANY test and it still passes. Thank you, very much.

Just a nit:

[  109s] python310-PySDL2.noarch: W: hidden-file-or-dir /usr/share/doc/packages/python310-PySDL2/doc/.DS_Store

I don’t think you want to leave these around.

a-hurst commented 2 years ago

@mcepl Ah yep, whoops! I thought I set up things to exclude those, but apparently it didn't work. I'll try to fix that for future releases.

a-hurst commented 2 years ago

Fix included in PySDL2 0.9.14, closing this!