Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use github actions to run tests and fix occuring issues #47

Merged
merged 12 commits into from
Nov 30, 2023

Conversation

K0lb3
Copy link
Contributor

@K0lb3 K0lb3 commented Dec 31, 2022

The main goal is to implement #21.
The commit fix Linux and mac geometry issues fixes #22 .

@K0lb3
Copy link
Contributor Author

K0lb3 commented Dec 31, 2022

Locally I got some issues with tests related to innerlight.mid.
FMOD errors when loading this midi file on my kali-linux, and the issue seems to be related to a wonky implementation on the site of FMOD itself according to this post.

@K0lb3
Copy link
Contributor Author

K0lb3 commented Dec 31, 2022

Cough, and I guess happy new year 🎉

@K0lb3
Copy link
Contributor Author

K0lb3 commented Dec 31, 2022

Currently, the github workflow:

  • Linux: errors due to midi loading
  • Windows: fails on various get info functions
  • Mac: runs fine, no errors

See https://github.com/K0lb3/pyfmodex/actions/runs/3814088863

As a side note,
legit the first time for me that MacOS is the first system to run fine.
Usually it's MacOS that is the special rainbow that has weird problems.

@K0lb3
Copy link
Contributor Author

K0lb3 commented Jan 1, 2023

Looking up the midi error,
the fmod core api reference says:

FMOD_ERR_PLUGIN_RESOURCE
A resource that the plugin requires cannot be found. (ie the DLS file for MIDI playback)

@K0lb3
Copy link
Contributor Author

K0lb3 commented Jan 1, 2023

Looking it up further, it looks like the lack of a dls soundback for Linux is the main problem; see Playing MIDI Files on Linux.

Which leaves following options to fix the tests on Linux:

  • somehow get a dls file to distribute
  • replace the midi related tests with fsb or bank tests
    • make a separate midi test for Windows and Mac, removing it for Linux

@K0lb3
Copy link
Contributor Author

K0lb3 commented Jan 1, 2023

I extracted the Piano1 of FreeFont2.sf2 and exported it as .dls,
which seems to have fixed the issue.

Now the Windows and Linux workflows only have following issues remaining:

2023-01-01T11:23:19.1350523Z FAILED tests/test_system.py::test_get_driver_info - pyfmodex.exceptions.FmodError: INVALID PARAM
2023-01-01T11:23:19.1350938Z FAILED tests/test_system.py::test_nm_drivers - assert 0 > 0
2023-01-01T11:23:19.1351249Z  +  where 0 = <pyfmodex.system.System object at 0x7f2f47c19ea0>.num_drivers
2023-01-01T11:23:19.1351699Z FAILED tests/test_system.py::test_get_record_driver_info - pyfmodex.exceptions.FmodError: INVALID PARAM
2023-01-01T11:23:19.1352134Z FAILED tests/test_system.py::test_record_num_drivers - assert 0 > 0
2023-01-01T11:23:19.1352588Z  +  where 0 = <pyfmodex.structobject.Structobject object at 0x7f2f474c09d0>.drivers
2023-01-01T11:23:19.1353300Z  +    where <pyfmodex.structobject.Structobject object at 0x7f2f474c09d0> = <pyfmodex.system.System object at 0x7f2f47c6bee0>.record_num_drivers
2023-01-01T11:23:19.1353882Z FAILED tests/test_system.py::test_is_recording - pyfmodex.exceptions.FmodError: INVALID PARAM

@K0lb3
Copy link
Contributor Author

K0lb3 commented Jan 1, 2023

Looks like the workflow instances for both systems are lacking audio out devices.
I'm trying to implement the things mentioned in actions/runner-images - Virtual sound device on Windows Instance

@tyrylu
Copy link
Owner

tyrylu commented Jan 1, 2023

Thank you for looking into this. I'll look into it sometimes the following week, e. g. we had a quite huge New year's party.

@K0lb3
Copy link
Contributor Author

K0lb3 commented Jan 3, 2023

Sounds good.

The only remaining thing that doesn't work yet is the recording on Linux,
which is related to the virtual audio setup.

=========================== short test summary info ============================
FAILED tests/test_channel.py::test_dsp_clock - assert 3072 > 5000
 +  where 3072 = <pyfmodex.structobject.Structobject object at 0x7f6b1f760820>.parent_clock
 +    where <pyfmodex.structobject.Structobject object at 0x7f6b1f760820> = <pyfmodex.channel.Channel object at 0x7f6b1f761540>.dsp_clock
FAILED tests/test_channel_group.py::test_dsp_clock - assert 3072 > 5000
 +  where 3072 = <pyfmodex.structobject.Structobject object at 0x7f6b1f70bfa0>.parent_clock
 +    where <pyfmodex.structobject.Structobject object at 0x7f6b1f70bfa0> = <pyfmodex.channel_group.ChannelGroup object at 0x7f6b1f70b9d0>.dsp_clock
FAILED tests/test_dsp_connection.py::test_input - pyfmodex.exceptions.FmodError: NOTREADY
=================== 3 failed, 252 passed, 1 skipped in 4.23s ===================

Windows and MacOS work already.

@tyrylu
Copy link
Owner

tyrylu commented Jan 3, 2023

Looks good. I have no idea whether we can get the DSP clock in some way for the test, yes, the number was chosen based on what was seen on Windows when writing the test.

@K0lb3
Copy link
Contributor Author

K0lb3 commented Jan 6, 2023

I found some header files for the linux base system,
so it might be possible to simply request the dsp clock from the system there.

I'm not really sure about how to fix the test_input test though.

@tyrylu
Copy link
Owner

tyrylu commented Jan 7, 2023

Feel free to fix the CI, we'll look at the recording tests then. But otherwise looks good, and I am not worried about bundling the libraries, I think I saw a message on the Fmod forum which explicitly allows this.

@K0lb3 K0lb3 marked this pull request as ready for review October 19, 2023 12:10
@K0lb3
Copy link
Contributor Author

K0lb3 commented Oct 19, 2023

I just noticed that I never made this draft into a real pr.
The issues still remain tho^^

@tyrylu
Copy link
Owner

tyrylu commented Oct 19, 2023

I'd just annotate the tests as expected failures and fix it later.

@K0lb3
Copy link
Contributor Author

K0lb3 commented Nov 24, 2023

A bit late, but done.
Currently the Mac machines seem to be a bit overloaded on Github, so the tests might fail due to the workflow not finding a machine in time -.-

@tyrylu
Copy link
Owner

tyrylu commented Nov 26, 2023

So, you consider this ready to merge, right? I do too, so, if yes, we can do it.

@K0lb3
Copy link
Contributor Author

K0lb3 commented Nov 27, 2023

Not yet, looks like the audio-setup for the Windows runners broke.

https://github.com/marketplace/actions/sound-ci-helper

would work, but breaks some more tests, which is a bit sub-optimal

K0lb3 added a commit to K0lb3/pyfmodex that referenced this pull request Nov 27, 2023
* PR tyrylu#47 - line ending fix

* Update test_dsp_connection.py
Copy link
Contributor Author

@K0lb3 K0lb3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good for me now.
I fixed the line endings of two tests as well now.

@K0lb3
Copy link
Contributor Author

K0lb3 commented Nov 27, 2023

The commit messages are a bit of a mess due to conflict handling,
so a squash merge might be for the best.

@tyrylu
Copy link
Owner

tyrylu commented Nov 27, 2023

Thanks for the work. I'll get to it after my work, so sometimes this evening, I guess.

@tyrylu tyrylu merged commit d87d93f into tyrylu:master Nov 30, 2023
@K0lb3
Copy link
Contributor Author

K0lb3 commented Dec 15, 2023

@tyrylu Since the pr also included some minor fixes, a minor version bump would be cool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pytest segfault
3 participants