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 added to p5.fft #611

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Test added to p5.fft #611

wants to merge 2 commits into from

Conversation

endurance21
Copy link
Collaborator

added tests for setInput method of p5.fft
added some lint fix to src/looper.js

@endurance21
Copy link
Collaborator Author

@therewasaguy please have a look

try {
fft.setInput(soundFile);
} catch (err) {
return done(err);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it catches an error , right now i am unsure of why we cant set sounfile as input , any suggestions @therewasaguy

Copy link
Contributor

Choose a reason for hiding this comment

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

@davepagurek any thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, I can't actually reproduce this error, it seems to just work. Maybe it was a browser bug that has since been fixed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I will try to reproduce this error, if it works we can merge this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

@davepagurek it was dropping these errors for me :

image

but when I added: let fft = new p5.FFT(); it got resolved, anything else seems fine to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh does this happen when running tests headless from the other open PR? I'm running npm run test and going to http://localhost:8000/test/?grep=FFT, which is what shows no errors.

Where are you adding the new p5.FFT() line? It also has that in a beforeEach, but maybe something is up with the asynchronous tests 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya I am getting that while running the test headless, I added the new line within the function, at the very beginning just like we did for oscillator = new p5.Oscillator();

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.

3 participants