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

Test result api fixes#60 #63

Merged
merged 2 commits into from
Aug 19, 2019
Merged

Test result api fixes#60 #63

merged 2 commits into from
Aug 19, 2019

Conversation

chendaniely
Copy link
Member

@chendaniely chendaniely commented Aug 14, 2019

This will be rebased after #62 gets merged.

Implements the changes for test_result

@chendaniely chendaniely mentioned this pull request Aug 14, 2019
@chendaniely
Copy link
Member Author

correct and incorrect should be used to return when all tests pass or not, respectively

R/glue.R Outdated Show resolved Hide resolved
#' 2/2 correct! Absolutely fabulous!.
#' @param glue_incorrect A glue string that returns the final correct message displayed.
#' Defaults to \code{getOption("gradethis_glue_correct_test")}, e.g.,
#' 1/2 correct! Try it again; next time's the charm!.
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing code and quotes.

\code{"1/2 correct! ... "}

Copy link
Member Author

Choose a reason for hiding this comment

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

Will make the change in #68

R/test_result.R Show resolved Hide resolved
R/test_result.R Outdated
return(evaluated_condi)

} else { # evaluating a fail_if condition # nolint
if (is.null(evaluated_condi)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this if/else can be replaced with

evaluated_condi <- evaluated_condi %||% graded(correct = TRUE, message = NULL)

A passing fail_if returns a graded(correct = FALSE, message = MESSAGE) object.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh you're right! I just wrote it super explicitly to not make my brain hurt.
I should update the docs about that too...

small doc changes for pkgdown

doc fixes

rebase/fix conflicts/fix docs for R CMD check

things to stash (broken)

test_result implementation + tests fixes #60

rename test files for consistency

doc updates (mainly test_result)

refactor conditional flip for loop to map call
@chendaniely chendaniely changed the base branch from doc_fixes to master August 14, 2019 23:49
@@ -1,4 +1,4 @@
#' Evaluates a condition
#' Evaluates a conditio
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing n

@@ -12,7 +12,10 @@
#'
#' TODO need to export when feature is implement in RStudio IDE
#' @noRd
<<<<<<< HEAD:archive/view_tutorial.R
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove

@chendaniely chendaniely merged commit 9cc3d6d into master Aug 19, 2019
@chendaniely chendaniely deleted the test_result_api_fixes#60 branch August 19, 2019 21:24
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