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

Make QuestionPresenterTest more realistic #2077

Merged
merged 1 commit into from
Nov 12, 2015

Conversation

floehopper
Copy link
Contributor

Since phrase lists are now only interpolated for question nodes, these tests
didn't make much sense. Especially as they were using the QuestionPresenter
with an Outcome node.

Also I'm not sure why the log statement was only being executed for outcome
nodes. If anything the opposite is now true and in any case the code in
SmartAnswer::I18nRenderer#value_for_interpolation will only ever be executed
for question nodes.

Expected observable changes

  • None

Since phrase lists are now *only* interpolated for question nodes, these tests
didn't make much sense. Especially as they were using the `QuestionPresenter`
with an `Outcome` node.

Also I'm not sure why the log statement was only being executed for outcome
nodes. If anything the opposite is now true and in any case the code in
`SmartAnswer::I18nRenderer#value_for_interpolation` will only ever be executed
for question nodes.
@chrisroos
Copy link
Contributor

Looks good to me.

@chrisroos chrisroos self-assigned this Nov 12, 2015
@chrisroos chrisroos added the LGTM label Nov 12, 2015
floehopper added a commit that referenced this pull request Nov 12, 2015
…ore-realistic

Make QuestionPresenterTest more realistic
@floehopper floehopper merged commit 314345b into master Nov 12, 2015
@floehopper floehopper deleted the make-question-presenter-test-more-realistic branch November 12, 2015 16: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