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

Fix SpeechAnnouncementListener example and add tests #1166

Merged
merged 1 commit into from Aug 3, 2018

Conversation

danesfeder
Copy link
Contributor

For the example of SpeechInstructionListener in EmbeddedNavigationActivity, we were modifying a SpeechAnnouncement created with a VoiceInstructionMilestone (this will always be the case).

Because it was create with the milestone, the milestone data was overriding the modification. I added some javadoc to clearly state that this will happen as well.

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class SpeechAnnouncementTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Test that AutoValue works as expected is not worth it IMO. Ultimately, it's like testing a setter. 👀 https://speakerdeck.com/guardiola31337/elegant-unit-testing-droidcon-berlin-2016?slide=68

@danesfeder
Copy link
Contributor Author

@Guardiola31337 this is testing custom build logic within the AutoValue class

private SpeechAnnouncement buildSpeechAnnouncement() {
VoiceInstructionMilestone milestone = voiceInstructionMilestone();
if (milestone != null) {
ssmlAnnouncement(milestone.getSsmlAnnouncement());
announcement(milestone.getAnnouncement());
return autoBuild();
} else {
return autoBuild();
}
}
}

Still not worth it you think?

@danesfeder danesfeder force-pushed the dan-speech-announcement-milestone branch from 13685e8 to ba4e66a Compare August 3, 2018 17:47
@danesfeder danesfeder merged commit 2f8fef0 into master Aug 3, 2018
@danesfeder danesfeder deleted the dan-speech-announcement-milestone branch August 3, 2018 18:07
@Guardiola31337 Guardiola31337 mentioned this pull request Aug 10, 2018
11 tasks
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.

None yet

2 participants