Skip to content

Commit

Permalink
refactor: improve readability and logic inside guess_where_config
Browse files Browse the repository at this point in the history 
Why:

- clearly outline the different cases: since now the user can add a golem-config, there can be conflicting cases between the default golem config and the user golem config (if e.g. the user forgets to delete the default)
- update the tests to check these different cases
- remark: test inside testthat should work without golem:: prefixes
  • Loading branch information
@ilyaZar
ilyaZar committed Aug 3, 2023
1 parent 90ee1ca commit a941ec1
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 52 deletions.
96 changes: 63 additions & 33 deletions R/config.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,48 +12,76 @@ guess_where_config <- function(
path = golem::pkg_path(),
file = "inst/golem-config.yml"
) {
# We'll try to guess where the path
# to the golem-config file is

# This one should be correct in 99% of the case
# If we don't change the default values of the params.
# => current directory /inst/golem-config.yml
ret_path <- fs_path(
path,
file
)
if (fs_file_exists(ret_path)) {
return(fs_path_abs(ret_path))
# We'll try to guess where the path to the golem-config file is. Since the
# user can now supply a user-defined golem-config there may be several cases
# to consider:

# 0. Firstly:
# Read from default and possible user specified locations:
ret_pth_def <- fs_path(path, file)
ret_pth_usr <- try_user_config_location(pth = path)
# Define booleans for different cases
CONFIG_DFLT_EXISTS <- fs_file_exists(ret_pth_def)
CONFIG_DFLT_MISSNG <- !CONFIG_DFLT_EXISTS
CONFIG_USER_EXISTS <- !is.null(ret_pth_usr) && (!identical(ret_pth_usr, ret_pth_def))
CONFIG_USER_MISSNG <- !CONFIG_USER_EXISTS
# Case I.
# The default config exists AND "R/app_config.R" does not provide any
# information about a possible user config (this one should be correct in 99%
# of the cases if no changes to the default param values are made).
# => read config from "inst/golem-config.yml"
if (CONFIG_DFLT_EXISTS && CONFIG_USER_MISSNG) {
return(fs_path_abs(ret_pth_def))
}

# Maybe for some reason we are in inst/
ret_path <- "golem-config.yml"
if (fs_file_exists(ret_path)) {
return(fs_path_abs(ret_path))
# Case II.
# The default config does not exists AND "R/app_config.R" provides information
# about a possible user config (this one should be correct if the user changed
# the location of the golem-config and properly deleted the (default) file in
# the default path "inst/golem-config.yml").
# => read path to user-config from argument to the app_sys()-call inside
# "R/app_config.R"
if (CONFIG_DFLT_MISSNG && CONFIG_USER_EXISTS) {
return(fs_path_abs(ret_pth_usr))
}
# Case III.
# The default config does exists AND "R/app_config.R" provides information
# about a possible user config which is found! (this occurs if the user
# changed the location/name of the golem-config, properly adjusted the
# corresponding line in "R/app_config.R" but forgot to delete the (default)
# config file in the default path "inst/golem-config.yml").
# => Throw error and prompt user to check whether default config or user
# config should be used.
if (CONFIG_DFLT_EXISTS && CONFIG_USER_EXISTS) {
msg_err <- paste0(
"It appears that two golem config files exist:\n",
"- the default 'inst/golem-config.yml'\n",
"- file read from an app_sys()-call in 'R/app_config.R'\n",
"=> Resolve via either of the two options:\n",
"1. KEEP USER-FILE: rename/delete default 'inst/golem-config.yml'\n",
"2. KEEP DEFAULT: change 'app_sys(...)' to 'app_sys('golem-config.yml')'.")
stop(msg_err)
}

# Trying with pkg_path
ret_path <- attempt({
# Case IV. All other "exotic" cases
# IV.A Maybe for some reason we are in inst/
ret_pth <- "golem-config.yml"
if (fs_file_exists(ret_pth)) {
return(fs_path_abs(ret_pth))
}
# IV.B Trying with pkg_path
ret_pth <- attempt({
fs_path(
golem::pkg_path(),
"inst/golem-config.yml"
)
})

if (
!is_try_error(ret_path) &&
fs_file_exists(ret_path)
) {
return(
fs_path_abs(ret_path)
)
if (!is_try_error(ret_pth) && fs_file_exists(ret_pth)) {
return(fs_path_abs(ret_pth))
}
ret_path <- try_user_config_location(pth = golem::pkg_path())
if (is.null(ret_path)) return(NULL)
if (fs_file_exists(ret_path)) {
return(fs_path_abs(ret_path))
if (fs_file_exists(ret_pth)) {
return(fs_path_abs(ret_pth))
}

# If all cases fail return NULL
return(NULL)
}
try_user_config_location <- function(pth) {
Expand All @@ -67,7 +95,9 @@ try_user_config_location <- function(pth) {
tmp_guess_text <- readLines("R/app_config.R")
tmp_guess_lines <- guess_lines_to_config_file(tmp_guess_text)
## -> early return if malformation i.e. no lines found that match app_sys(...)
if (is.null(tmp_guess_lines)) return(NULL)
if (is.null(tmp_guess_lines)) {
return(NULL)
}

# III. Collapse character-text found into a single char from which to retrieve
# path! This is important if the path is a (complicated) multi-line path.
Expand Down
54 changes: 35 additions & 19 deletions tests/testthat/test-config.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ test_that("config works", {
test_that("golem-config.yml can be renamed and moved to another location", {

path_dummy_golem <- tempfile(pattern = "dummygolem")
golem::create_golem(
create_golem(
path = path_dummy_golem,
open = FALSE
)
Expand All @@ -89,8 +89,8 @@ test_that("golem-config.yml can be renamed and moved to another location", {

## The default config path is returned
expect_equal(
golem:::guess_where_config(),
golem:::fs_path_abs(file.path(
guess_where_config(),
fs_path_abs(file.path(
path_dummy_golem,
"inst/golem-config.yml"
))
Expand All @@ -105,25 +105,27 @@ test_that("golem-config.yml can be renamed and moved to another location", {
from = "inst/golem-config.yml",
to = "inst/config/golem.yml"
)
## III. Remove old config file
file.remove(
"inst/golem-config.yml"
)
## IV Save content of default config to restore and for final test later
## III Save content of default config to restore and for final test later
tmp_app_config_default <- readLines("R/app_config.R")
# V.A User alters correct line in app_config.R: different path AND filename!
# IV.A User alters correct line in app_config.R: different path AND filename!
tmp_app_config_test_01 <- readLines("R/app_config.R")
tmp_app_config_test_01[36] <- " file = app_sys(\"config/golem.yml\")"
writeLines(tmp_app_config_test_01, "R/app_config.R")
# V.A The updated config is found
# Keeping two configs (user and default) is forbidden i.e. we expect an error
expect_error(guess_where_config())
## Remove old config file
file.remove(
"inst/golem-config.yml"
)
# IV.A The updated config is found
expect_equal(
golem:::guess_where_config(),
golem:::fs_path_abs(file.path(
guess_where_config(),
fs_path_abs(file.path(
path_dummy_golem,
"inst/config/golem.yml"
))
)
## V.B app_config.R has a multi-line statement
## IV.B app_config.R has a multi-line statement
## - > e.g. because of grkstyle formatting or long path names
tmp_app_config_test_02 <- tmp_app_config_default
tmp_max_lines_config <- length(tmp_app_config_test_02)
Expand All @@ -133,20 +135,34 @@ test_that("golem-config.yml can be renamed and moved to another location", {
")",
tmp_app_config_test_02[37:tmp_max_lines_config])
writeLines(tmp_app_config_test_02, "R/app_config.R")
# V.B The updated config with multi-line app_sys()-call is read correctly
# IV.B The updated config with multi-line app_sys()-call is read correctly
expect_equal(
golem:::guess_where_config(),
golem:::fs_path_abs(file.path(
guess_where_config(),
fs_path_abs(file.path(
path_dummy_golem,
"inst/config/golem.yml"
))
)
## V.C app_config.R has a malformatted app_sys(...) call
## IV.C app_config.R has a malformatted app_sys(...) call
## -> e.g. because of grkstyle formatting or long path names
tmp_app_config_test_03 <- tmp_app_config_default
## malformation = typo: app_syss() instead of app_sys()
tmp_app_config_test_03[36] <- " file = app_syss(\"config/golem.yml\")"
writeLines(tmp_app_config_test_03, "R/app_config.R")
# V.C The updated config with multi-line app_sys()-call is read correctly
expect_null(golem:::guess_where_config())
# IV.C The updated config with multi-line app_sys()-call is read correctly
expect_null(guess_where_config())
# V. And finally if default config-file is missing AND no valid app_config.R
# V.A test that default sub-function returns path
tmp_app_config_test_03[36] <- " file = app_sys(\"config/golem.yml\")"
writeLines(tmp_app_config_test_03, "R/app_config.R")
expect_equal(
try_user_config_location(pkg_path()),
fs_path_abs(file.path(
path_dummy_golem,
"inst/config/golem.yml"
))
)
# V.B test that default sub-function fails to return path and gives NULL
file.remove("R/app_config.R")
expect_null(try_user_config_location(pkg_path()))
})

0 comments on commit a941ec1

Please sign in to comment.