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

testsuite: Fix TestTempoClock:test_nextTimeOnGrid_negativePhaseWraps #4334

Merged
merged 1 commit into from
Jun 8, 2019
Merged

testsuite: Fix TestTempoClock:test_nextTimeOnGrid_negativePhaseWraps #4334

merged 1 commit into from
Jun 8, 2019

Conversation

jamshark70
Copy link
Contributor

Purpose and Motivation

TestTempoClock:test_nextTimeOnGrid_negativePhaseWraps is badly written:

  • It uses a condition, hang and unhang where nothing is happening except a wait. Should remove the extra complexity.

  • It fails when running the entire TestTempoClock because, in that case, the beats when entering the test are non-zero. It should set the beats explicitly.

Types of changes

  • Bug fix

To-do list

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

@jamshark70 jamshark70 added the comp: testing UnitTest class, refactors of existing tests, etc.; don't use if just adding tests as part of a PR label Feb 26, 2019
@patrickdupuis patrickdupuis added this to the 3.10.3 milestone Jun 8, 2019
nhthn
nhthn approved these changes Jun 8, 2019
View reviewed changes
Copy link
Contributor

@nhthn nhthn left a comment

Choose a reason for hiding this comment

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

thanks!

@nhthn nhthn merged commit e7f980a into supercollider:develop Jun 8, 2019
@patrickdupuis patrickdupuis mentioned this pull request Jun 8, 2019
Merged
4 tasks
@joshpar joshpar mentioned this pull request Jul 16, 2019
Closed
@mossheim mossheim added comp: testing 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 UnitTest class, refactors of existing tests, etc.; don't use if just adding tests as part of a PR labels Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrickdupuis patrickdupuis patrickdupuis approved these changes

@nhthn nhthn nhthn 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
3.10.3
Development

Successfully merging this pull request may close these issues.
4 participants
@jamshark70 @nhthn @patrickdupuis @mossheim