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

Add tests for engine #47

Merged
merged 3 commits into from
Jul 4, 2020
Merged

Add tests for engine #47

merged 3 commits into from
Jul 4, 2020

Conversation

amclain
Copy link
Member

@amclain amclain commented Jul 2, 2020

This PR adds some unit tests for Engine. I intentionally omitted testing next_animation and previous_animation since those functions are slated to be removed from Engine in #36. Hopefully these tests aid in that refactor by solidifying some of the expected behavior of Engine.

I should note that I also moved the responsibility of choosing the initial animation. Instead having the engine decide this, it now accepts an initial_animation. Not only did this make testing easier, but it is probably a better design when building an app with Xebow as well. This will let a developer choose the initial animation easier.

@amclain amclain added the refactor Refactoring code or tech debt repayment label Jul 2, 2020
@amclain amclain requested a review from doughsay July 2, 2020 22:15
@amclain amclain self-assigned this Jul 2, 2020
@amclain amclain force-pushed the tech-debt/add-engine-tests branch from 0eb786a to 7993996 Compare July 2, 2020 22:50
Also supply engine with an initial animation.
@amclain amclain force-pushed the tech-debt/add-engine-tests branch from 7993996 to 00a9e1c Compare July 3, 2020 19:02
@amclain amclain merged commit d0595ad into master Jul 4, 2020
@amclain amclain deleted the tech-debt/add-engine-tests branch July 4, 2020 06:17
@amclain amclain restored the tech-debt/add-engine-tests branch July 4, 2020 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring code or tech debt repayment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants