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

More fun with JUnit 5 #73

Merged
merged 4 commits into from
Sep 26, 2023
Merged

More fun with JUnit 5 #73

merged 4 commits into from
Sep 26, 2023

Conversation

juleskers
Copy link
Collaborator

I've been doing some code-coverage-guided test-writing this weekend.
This unveiled some gaps in my previous Melody and SongPlayer tests.

I've also started working my way through Piano.
It feels a bit dirty, because I had to make some edits to "production" logic, in order to make it testable.
I've been taught that's normally a big no-no (at work).
For PianOli, I'm willing to break that rule, since I think it's an intermediate step on the way to move sound-playing to PianoCanvas.
The Canvas does all the other Android-API-related stuff, while Piano is pure coordinate/state-handling if you ignore sounds.

I'm thinking Piano will end up with some kind of Listener-pattern, where both the Canvas and the AppConfigTrigger listen for keydown-events. However, it's still crystallizing in my brain.

In the meanwhile, I wanted to publish these additional tests, since they're useful in and of themselves.

@nicolasbrailo
Copy link
Owner

Thanks @juleskers
One suggestion for a test: can we add one to test the stability of the sizes? I noticed the size of the keys in the app change after the first keypress on the latest version of the app, and I'm not sure why or when we introduced that

@nicolasbrailo nicolasbrailo merged commit 4f7a6a8 into master Sep 26, 2023
@juleskers juleskers deleted the more-coverage branch October 18, 2023 18:13
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