Skip to content

Commit

Permalink
Drop non-list HTML dependencies from exercise results
Browse files Browse the repository at this point in the history
Fixes #484
  • Loading branch information
gadenbuie committed Feb 3, 2021
1 parent 1ac35b1 commit 3b3ff2c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 7 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ learnr (development version)
* Fail gracefully when unable to open an indexedDB store (e.g. in cross-origin iframes in Safari). ([#417](https://github.com/rstudio/learnr/issues/417)).
* When a quiz's question or answer text are not characters, e.g. HTML, `htmltools` tags, numeric, etc., they are now cast to characters for the displayed answer text and the quiz's default loading text ([#450](https://github.com/rstudio/learnr/pull/450)).
* The `envir_prep` environment used in exercise checking now captures the result of both global and exercise-specific setup code, representing the environment in which the user code will be evaluated as described in the documentation. We also ensure that `envir_result` (the environment containing the result of evaluating global, setup and user code) is a sibling of `envir_prep`. ([#480](https://github.com/rstudio/learnr/pull/480))
* HTML dependencies of exercises run by users now excludes dependencies created with `htmltools::tags$head()`. (thanks @andysouth, [#484](https://github.com/rstudio/learnr/issues/484))

learnr 0.10.1
===========
Expand Down
15 changes: 8 additions & 7 deletions R/exercise.R
Original file line number Diff line number Diff line change
Expand Up @@ -414,11 +414,11 @@ render_exercise <- function(exercise, envir) {
})

# Copy in a full clone `envir_prep` before running user code in `envir_result`
# By being a sibling to `envir_prep` (rather than a dependency),
# alterations to `envir_prep` from eval'ing code in `envir_result`
# By being a sibling to `envir_prep` (rather than a dependency),
# alterations to `envir_prep` from eval'ing code in `envir_result`
# are much more difficult
envir_result <- duplicate_env(envir_prep)

# Now render user code for final result
rmarkdown::render(
input = rmd_file_user,
Expand Down Expand Up @@ -578,12 +578,13 @@ is_error_result <- function(x) {

filter_dependencies <- function(dependencies) {
# purge dependencies that aren't in a package (to close off reading of
# artibtary filesystem locations)
# arbitrary filesystem locations)
Filter(x = dependencies, function(dependency) {
if (!is.null(dependency$package)) {
if (!is.list(dependency)) {
FALSE
} else if (!is.null(dependency$package)) {
TRUE
}
else {
} else {
! is.null(tryCatch(
rprojroot::find_root(rprojroot::is_r_package,
path = dependency$src$file),
Expand Down
23 changes: 23 additions & 0 deletions tests/testthat/test-exercise.R
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,26 @@ test_that("serialized exercises produce equivalent evaluate_exercise() results",
env_vals(ex_eval_rmote$feedback$checker_args$envir_result)
)
})

# filter_dependencies() ---------------------------------------------------

test_that("filter_dependencies() excludes non-list knit_meta objects", {
ex <- mock_exercise(
user_code =
"htmltools::tagList(
htmltools::tags$head(htmltools::tags$style(\".leaflet-container {backround:#FFF}\")),
idb_html_dependency()
)"
)

ex_res <- expect_silent(withr::with_tempdir(render_exercise(ex, new.env())))

ex_res_html_deps <- htmltools::htmlDependencies(ex_res$html_output)
# The head(style) dependency is dropped because it's not from a package
expect_equal(length(ex_res_html_deps), 1L)
# But we keep the dependency that came from a pkg
expect_equal(
ex_res_html_deps[[1]],
idb_html_dependency()
)
})

0 comments on commit 3b3ff2c

Please sign in to comment.