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

test:adding test for synthutils.js #4210

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Shyam-Raghuwanshi
Copy link
Contributor

@Shyam-Raghuwanshi Shyam-Raghuwanshi commented Dec 30, 2024

adding test for all the function of synthutils.js
#4124

locally test my tests

npm install tone
jest synthutils.test.js

@Shyam-Raghuwanshi
Copy link
Contributor Author

Shyam-Raghuwanshi commented Dec 30, 2024

hi @walterbender I was working on this for a week I added tests for all functions. Please take a look or If you want to some changes I am happy to work on it

@BeNikk
Copy link
Contributor

BeNikk commented Jan 21, 2025

Hey, the tests look good but there are some issues
a-Im not sure why you changed the original synthutils file,
also please look at how the files are we exporting the files (there's an if condition, otherwise exporting modules like this throws reference errors)

And i think we are not supposed to change the package.json/lock files

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.

2 participants