-
Notifications
You must be signed in to change notification settings - Fork 240
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
Move error checking from render_exercise()
to evaluate_exercise()
#544
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start and I really like that we'll collect all of the checking logic in one place in evaluate_exercise()
rather than render_exercise()
.
The tests are currently written around the error checking logic being inside render_exercise()
but now that we've moved that out we should
- Update the error-checking tests so they call and reference
evaluate_exercise()
- Add tests the explicitly check that we get the correct expected errors from
render_exercise()
For next steps, we can update the temp directory logic so that we stay in the temp directory while evaluating the checker functions as well. So rather than |
Merge commit '14d679a91f7afef36442729c8ad5a485918fad98' Conflicts: R/exercise.R
Merge branch 'evaluate-conditions' of https://github.com/rossellhayes/learnr into evaluate-conditions # Conflicts: # R/exercise.R
We may want to use it in other places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thank you!
This is the first step in the process of moving error handling code out of
render_exercise()
and intoevaluate_exercise()
Fixes #481