Closed dvzrv closed 3 years ago
Hmm well we run all the tests on every build so def something about your setup That test will fail if the wavetable oscillator can’t find wavetables. You sure that see is working? I our next release I have en environment var to override the data path which will make things much easier.
Thanks for the hint!
Maybe the hack for the local test is not enough and I need to modify more changes than the sed -e 's|/usr/share/Surge|resources/data|' -i src/common/SurgeStorage.cpp
Do note, that this affects only a test binary, not the packaged binary! I keep these two separated :)
If you look in the azure pipelines file you can see how we solve this in ci. Basically we stage the resources to a tmp dir and point xdghome at it. Maybe same would work?
In our xt branch there’s a variable for direct override but not in 19
Not a bug, then?
@baconpaul okay, using XDG_DATA_HOME is of course much more straight forward for setting up the tests. Unfortunately I still get the same error:
# surge-headless: 1.9.0 built: 0 0
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
surge-headless is a Catch v2.9.2 host application.
Run with -? for options
-------------------------------------------------------------------------------
Every Oscillator Plays
Oscillator type Wavetable
-------------------------------------------------------------------------------
/build/surge/src/surge-1.9.0/src/headless/UnitTestsDSP.cpp:781
...............................................................................
/build/surge/src/surge-1.9.0/src/headless/UnitTestsDSP.cpp:806: FAILED:
REQUIRE( sumAbsOut > 1 )
with expansion:
0.0f > 1
===============================================================================
test cases: 59 | 58 passed | 1 failed
assertions: 4641905 | 4641904 passed | 1 failed
Well that is perplexing
We don’t get that break on ubuntu
One thought is that will use a default wavetable and I wonder if that default could from a fs scan order or some such. That’s a sloppy test of course - we should force a repeatable wavetable. But may be it. If I share a little micro patch here would you be able to try it easily? But probably not today.
thanks for your patience and testing!
If I share a little micro patch here would you be able to try it easily? But probably not today.
Yes, sure! :)
diff --git a/src/headless/UnitTestsDSP.cpp b/src/headless/UnitTestsDSP.cpp
index b87ceb4f..72f2fb83 100644
--- a/src/headless/UnitTestsDSP.cpp
+++ b/src/headless/UnitTestsDSP.cpp
@@ -792,6 +792,25 @@ TEST_CASE("Every Oscillator Plays", "[dsp]")
for (int q = 0; q < 10; ++q)
surge->process();
+
+ int idx = 0;
+ bool got = false;
+ for (auto q : surge->storage.wt_list)
+ {
+ if (q.name == "Sine Power HQ")
+ {
+ got = true;
+ surge->storage.getPatch().scene[0].osc[0].wt.queue_id = idx;
+ }
+ idx++;
+ }
+ REQUIRE(got);
+ for (int q = 0; q < 10; ++q)
+ surge->process();
+
+ REQUIRE(std::string(surge->storage.getPatch().scene[0].osc[0].wavetable_display_name) ==
+ "Sine Power HQ");
+
float sumAbsOut = 0;
surge->playNote(0, 60, 127, 0);
for (int q = 0; q < 100; ++q)
If you apply that patch, does it work? That forces you to load one of the factor wavetables, makes sure it is loaded
My theory is we choose the first wavetable by scan by default. Something about your install is picking one which is silent in the generator at default (either it is a growing wavetable or a one shot or some such). This will force you to something which, in my system, makes sound.
I plan to merge this in any case - makes the test far better - but would love to know if it fixes your problem first.
Thanks!
So I pushed that diff just now to our next-release branch (xt-alpha) and all our CI tests passed. Curious if it fixes the problem for you also.
If not, then next step is to bring up surge and see if the wavetable oscillator actually makes noise in your system!
That does indeed fix the test. Thanks!
I am suspecting, that if this is a sorting issue, it is probably introduced by the filesystem. I have e.g. run into issues with games on Linux (Civilization V has this problem), that were created to implicitly rely on a filesystem such as NTFS to sort files for loading in a very specific order. This leads e.g. steam to only support this game when run from an ext4 partition (as with that filesystem they can emulate the behavior), but e.g. btrfs is not supported, as the game will just crash as soon as it loads files in the wrong order.
I guess the take away is: It is good to be explicit! :)
Thanks again for looking into fixing this!
Yeah we don’t rely on fs order per se but the default wt is the first in fs order so we got unlucky here. I’ll merge the fix into both branches today. Thanks!
Oh hey @dvzrv I was just looking at https://github.com/archlinux/svntogit-community/blob/packages/surge/trunk/PKGBUILD quickly. A couple of minor things
Thanks!
hey @baconpaul
Thanks for the drive-by review! :D
Awesome thanks - I just moved 1701 into our next milestone and will add that flag. Appreciated!
Bug Description: When running the unit tests for 1.9.0 the UnitTestsDSP fails.
Surge Version This information is found on the About screen, which you get to from the bottom right menu
Reproduction Steps: Steps to reproduce the behavior:
build-test/surge-headless
:Expected Behavior: All unit tests pass
Screenshots: n/a
Computer Information (please complete the following information):
Additional Information: