synthutils test is broken

[walterbender]

Description

The changes to the volume code changes and seems to have broken the unit test

[walterbender]

@falgun143 can you please investigate

[falgun143]

@walterbender sure. @omsuneri can I work on this ?

[omsuneri]

@falgun143 yaa you can but follow the guide while changing any test

[omsuneri]

@falgun143 I ll suggest you to first investigate the changes you did in #4396 and try to resolve from there without changing the actual test cases
Cause it's very difficult and poor idea to change every failing test case after any pr merges
Either changing the pr makes more sense

[Harshit-7373]

@walterbender Not only synthutils test is broken but their are many other tests failing.

1. 3 test suites are failing out of 46.
2. 10 tests are failed out of 682.

[omsuneri]

Thanks @Harshit-7373 for pointing it out but volumeaction.js test is resolved in the #4422 also the errors with synthutils.js i m resolving right now will be raising a PR for the same soon

[Harshit-7373]

@omsuneri I guess that PR is not merged yet because i am still getting error. Can you tell me something about it?

[omsuneri]

@omsuneri I guess that PR is not merged yet because i am still getting error. Can you tell me something about it?

yes its under review will be merged soon.

[falgun143]

@omsuneri what are the other test files you are working on?

[omsuneri]

@falgun143 you can take turtle-singer.js failing as i m not resolving it also try to add some more edge case tests

[falgun143]

@omsuneri alright I will take turtle-singer.js and write some checks in the .js files in which I made changes.

[omsuneri]

@falgun143 sure also dont try to alter the root file functions as well

[falgun143]

@walterbender, the reason why the synthutils test passes—apart from volume blocks—is purely luck. There is a disconnect between what we are calculating and what we are testing.
1.
Code Snippet:

BUGS:

We are calculating values for the flute but testing for the piano. As a result, Tone.gainToDb() gets called twice:

First, from this line:
setVolume("turtle1", "flute", 80);

2nd from thisSecond, from this calculation:
const expectedDb = Tone.gainToDb(nv / 100);

And now the Tone.gainToDb() will recieve two values and if in this function . expect(Tone.gainToDb).toHaveBeenCalledWith(0.96);
we pass in any of the two values which Tone.gainToDv recieved the test will pass. So to avoid this we have test and calulate for the same instrument.

2.

Similarly we are passing in guitar as agrs, And expecting instruments[turtle]["electronic synth"]).toBeInstanceOf(Tone.PolySynth)
and the reason this test works is that the createDefaultSynth() function, which runs earlier, has already set instruments[turtle]["electronic synth"].

I have added a few checks in my .js file to improve the robustness of the code. I will also make changes to synthutils.test.js and turtle.test.js.

I will raise a PR soon—I was a bit busy over the last 6 days.