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.

Sign up for GitHub

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

Jump to bottom

test: add test for audio rate controls #5304

Merged
merged 5 commits into from
Aug 7, 2021
Merged

test: add test for audio rate controls #5304

merged 5 commits into from
Aug 7, 2021

Conversation

telephon
Copy link
Member

@telephon telephon commented Dec 26, 2020 •
edited
Loading

Purpose and Motivation

This makes sure that node order doesn't matter for audio rate mapping, up to block delay.

To-do list

  • Code is tested
  • All tests are passing
  • This PR is ready for review

@mossheim mossheim added comp: testing UnitTest class, refactors of existing tests, etc.; don't use if just adding tests as part of a PR and removed comp: testing labels Mar 7, 2021
@dyfer
Copy link
Member

dyfer commented Aug 3, 2021

Thanks @telephon and sorry for the delay
Could you please rebase your PR on the latest develop? Once you do, the added tests would automatically be run in our CI.

@telephon telephon force-pushed the topic/test-audio-control-mapping-synth-order branch from 3da7a02 to e4c5606 Compare August 4, 2021 21:00
@telephon
Copy link
Member Author

telephon commented Aug 4, 2021

@dyfer good like this?

@dyfer
Copy link
Member

dyfer commented Aug 5, 2021 •
edited
Loading

The rebase is good, thanks!
One of the tests fails: test_map_with_control_rate
Not sure if I understand correctly, it seems that there is value discrepancy (possibly because of .kr -> .ar conversion - trigger arriving before the end of the control period?). Since you designed these tests you probably have a better idea what's the best way to address this.

@telephon
Copy link
Member Author

telephon commented Aug 6, 2021

Just a precision error. Should be better now.

@dyfer
Copy link
Member

dyfer commented Aug 6, 2021

Thanks! This still fails on macOS, as you can see in the CI. .asInteger truncates the floating point, one way to address this is to .round first I think?

@telephon
Copy link
Member Author

telephon commented Aug 7, 2021

OK, hope this works now.

dyfer
dyfer approved these changes Aug 7, 2021
View reviewed changes
Copy link
Member

@dyfer dyfer left a comment

Choose a reason for hiding this comment

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

Thanks!

@dyfer dyfer merged commit 8ff17af into supercollider:develop Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dyfer dyfer dyfer approved these changes

Assignees
No one assigned
Labels
comp: testing UnitTest class, refactors of existing tests, etc.; don't use if just adding tests as part of a PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@telephon @dyfer @mossheim