Skip to content

Commit b363ccc

Browse files
florisvdhflorisvdh
authored
Merge pull request #197 from r-spatial/tests_newer_qgis
Fix + test for optionally detecting and proposing newer QGIS (win/mac)
2 parents 33c8de4 + 885cdf3 commit b363ccc

File tree

7 files changed

+250
-23
lines changed

7 files changed

+250
-23
lines changed

‎DESCRIPTION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Package: qgisprocess
22
Title: Use 'QGIS' Processing Algorithms
3-
Version: 0.2.0.9001
3+
Version: 0.2.0.9002
44
Authors@R: c(
55
person("Dewey", "Dunnington", , "dewey@fishandwhistle.net", role = "aut",
66
comment = c(ORCID = "0000-0002-9415-4582", affiliation = "Voltron Data")),

‎NEWS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# qgisprocess (development version)
22

33
- More consistent and intuitive handling of JSON input / output user settings (#195, #196; see `?qgis_using_json_output`).
4+
- Fix bug in support for environment variable `R_QGISPROCESS_DETECT_NEWER_QGIS` (#197).
45

56
# qgisprocess 0.2.0
67

‎R/qgis-configure.R‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,9 @@ qgis_configure <- function(quiet = FALSE, use_cached_data = FALSE) {
145145

146146
# CACHE CONDITION: qgis_process is indeed available in the cached path
147147

148+
testopt <- getOption("qgisprocess.test_skip_path_availability_check")
148149
outcome <- try(qgis_run(path = cached_data$path), silent = TRUE)
149-
if (inherits(outcome, "try-error")) {
150+
if (inherits(outcome, "try-error") && !isTRUE(testopt)) {
150151
if (quiet) packageStartupMessage()
151152
packageStartupMessage(
152153
glue(
@@ -162,31 +163,24 @@ qgis_configure <- function(quiet = FALSE, use_cached_data = FALSE) {
162163
# environment variable/option to automatically switch to a newer
163164
# available QGIS version
164165
if (is_windows() || is_macos()) {
165-
opt <- getOption(
166-
"qgisprocess.detect_newer_qgis",
167-
Sys.getenv("R_QGISPROCESS_DETECT_NEWER_QGIS")
166+
opt <- resolve_flag_opt(
167+
option_name = "qgisprocess.detect_newer_qgis",
168+
envvar_name = "R_QGISPROCESS_DETECT_NEWER_QGIS"
168169
)
169-
assert_that(
170-
assertthat::is.flag(opt) ||
171-
(assertthat::is.string(opt) && opt %in% c("", "TRUE", "FALSE", "true", "false")),
172-
msg = "Option 'qgisprocess.detect_newer_qgis' must be 'TRUE' or 'FALSE'."
173-
)
174-
if (identical(opt, "")) opt <- NA
175-
opt || grepl("TRUE|true", opt)
176-
177170
first_qgis <- qgis_detect_paths()[1]
178171
newer_available <- !is.na(extract_version_from_paths(first_qgis)) &&
179172
!identical(cached_data$path, first_qgis)
180173

181-
if (isTRUE(opt) && isTRUE(newer_available) && interactive()) {
174+
if (opt && isTRUE(newer_available) && rlang::is_interactive()) {
182175
packageStartupMessage()
183176
packageStartupMessage(glue(
184177
"A newer QGIS installation seems to be available: ",
185178
"{extract_version_from_paths(first_qgis)}."
186179
))
187180
answer <- ""
188181
while (!grepl("^[Yy](?:[Ee][Ss])?$|^[Nn](?:[Oo])?$", answer)) {
189-
answer <- readline("Do you want to try it and rebuild the cache? (y/n) ")
182+
answer <- getOption("qgisprocess.test_try_new_qgis") %||%
183+
readline("Do you want to try it and rebuild the cache? (y/n) ")
190184
}
191185
if (grepl("^[Yy]", answer)) {
192186
newer_ok <- FALSE

‎R/qgis-help.R‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ qgis_using_cached_help <- function() {
227227
Sys.getenv("R_QGISPROCESS_USE_CACHED_HELP", "true")
228228
)
229229

230-
isTRUE(opt) || identical(opt, "true") || identical(opt, "TRUE")
230+
resolve_flag_opt(opt)
231231
}
232232

233233
help_cache_file <- function(algorithm, json) {

‎R/qgis-state.R‎

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ qgis_using_json_input <- function() {
317317
!is.null(qgis_version()) &&
318318
(package_version(qgis_version(full = FALSE)) >= "3.23.0")
319319
} else {
320-
json_input_is_set <- isTRUE(opt) || identical(opt, "true") || identical(opt, "TRUE")
320+
json_input_is_set <- resolve_flag_opt(opt)
321321
if (
322322
json_input_is_set &&
323323
!is.null(qgis_version()) &&
@@ -395,6 +395,55 @@ readopt <- function(option_name, envvar_name) {
395395
}
396396

397397

398+
399+
#' Resolve a boolean option or environmental variable to TRUE, FALSE or (optionally) NA
400+
#'
401+
#' @param value A result as obtained by [readopt()].
402+
#' @param keep_NA Return NA if option and env var are empty?
403+
#' (i.e. `NULL` and `""` respectively).
404+
#' The default (`FALSE`) will return `FALSE`.
405+
#'
406+
#' @noRd
407+
#'
408+
#' @keywords internal
409+
resolve_flag_opt <- function(
410+
value = readopt(option_name, envvar_name),
411+
option_name = NULL,
412+
envvar_name = NULL,
413+
keep_NA = FALSE
414+
) {
415+
if (missing(value)) {
416+
assert_that(
417+
!missing(option_name),
418+
!missing(envvar_name),
419+
msg = paste(
420+
"Both 'option_name' and 'envvar_name' must be provided if",
421+
"'value' is missing."
422+
)
423+
)
424+
}
425+
if (!missing(option_name)) assert_that(is.string(option_name))
426+
if (!missing(envvar_name)) assert_that(is.string(envvar_name))
427+
assert_that(is.flag(keep_NA))
428+
opt <- value
429+
assert_that(
430+
is.flag(opt) ||
431+
(is.string(opt) && opt %in% c("", "TRUE", "FALSE", "true", "false")),
432+
msg = glue("Option '{option_name %||% \"\"}' must be 'TRUE' or 'FALSE'.")
433+
)
434+
if (keep_NA) {
435+
if (identical(opt, "")) opt <- NA
436+
is.logical(opt) && length(opt) == 1 && opt
437+
} else {
438+
isTRUE(opt)
439+
} ||
440+
identical(opt, "true") ||
441+
identical(opt, "TRUE")
442+
}
443+
444+
445+
446+
398447
#' Handle an explicitly set 'use_json_output'
399448
#'
400449
#' The `qgisprocess.use_json_output` option or the
@@ -416,9 +465,7 @@ readopt <- function(option_name, envvar_name) {
416465
#' @noRd
417466
#' @keywords internal
418467
resolve_explicit_json_output <- function(json_output_setting, qgis_version) {
419-
json_output_is_set <- isTRUE(json_output_setting) ||
420-
identical(json_output_setting, "true") ||
421-
identical(json_output_setting, "TRUE")
468+
json_output_is_set <- resolve_flag_opt(json_output_setting)
422469
# with JSON INput EXPLICITLY set as TRUE, always use JSON output if the
423470
# version requirement is met (it is how 'qgis_process run' works, so
424471
# better do that throughout the package)
@@ -442,9 +489,7 @@ resolve_explicit_json_output <- function(json_output_setting, qgis_version) {
442489
#' @keywords internal
443490
json_input_set_and_acceptable <- function(qgis_version) {
444491
opt_json_input <- readopt_json_input()
445-
(isTRUE(opt_json_input) ||
446-
identical(opt_json_input, "true") ||
447-
identical(opt_json_input, "TRUE")) &&
492+
resolve_flag_opt(opt_json_input) &&
448493
!is.null(qgis_version) &&
449494
package_version(qgis_version) >= "3.23.0"
450495
}

‎tests/testthat/test-qgis-configure.R‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,134 @@ test_that("qgis_configure() works OK if cache condition 'use_json_output' unmet"
262262

263263

264264

265+
test_that("qgis_configure() works OK with qgisprocess.detect_newer_qgis option or envvar", {
266+
skip_if_not(has_qgis())
267+
skip_if_not(is_windows() || is_macos())
268+
version <- as.character(utils::packageVersion("qgisprocess"))
269+
cache_data_file <- file.path(
270+
rappdirs::user_cache_dir("R-qgisprocess"),
271+
glue("cache-{version}.rds")
272+
)
273+
rlang::local_interactive()
274+
withr::local_options(list(
275+
qgisprocess.test_skip_path_availability_check = TRUE,
276+
qgisprocess.detect_newer_qgis = TRUE
277+
))
278+
local_mocked_bindings(
279+
qgis_detect_paths = function(...) c(
280+
"C:/Program Files/QGIS 3.30.0/bin/qgis_process-qgis-ltr.bat",
281+
"C:/Program Files/QGIS 3.28.6/bin/qgis_process-qgis-ltr.bat"
282+
)
283+
)
284+
withr::defer(
285+
saveRDS(
286+
list(
287+
path = qgis_path(),
288+
version = qgis_version(),
289+
algorithms = qgis_algorithms(),
290+
plugins = qgis_plugins(),
291+
use_json_output = qgis_using_json_output()
292+
),
293+
cache_data_file
294+
)
295+
)
296+
297+
# answering 'yes' triggers reconfiguration with newer version
298+
withr::local_options(qgisprocess.test_try_new_qgis = "yes")
299+
saveRDS(
300+
list(
301+
path = "C:/Program Files/QGIS 3.28.6/bin/qgis_process-qgis-ltr.bat",
302+
version = "3.28.6-xxx",
303+
algorithms = qgis_algorithms(),
304+
plugins = qgis_plugins(),
305+
use_json_output = qgis_using_json_output()
306+
),
307+
cache_data_file
308+
)
309+
expect_message(
310+
capture.output(qgis_configure(use_cached_data = TRUE), type = "message"),
311+
"A newer QGIS installation seems to be"
312+
)
313+
314+
# answering 'no' triggers another message
315+
withr::local_options(qgisprocess.test_try_new_qgis = "no")
316+
saveRDS(
317+
list(
318+
path = "C:/Program Files/QGIS 3.28.6/bin/qgis_process-qgis-ltr.bat",
319+
version = "3.28.6-xxx",
320+
algorithms = qgis_algorithms(),
321+
plugins = qgis_plugins(),
322+
use_json_output = qgis_using_json_output()
323+
),
324+
cache_data_file
325+
)
326+
expect_message(
327+
capture.output(qgis_configure(use_cached_data = TRUE), type = "message"),
328+
"if you don't want to autodetect QGIS version updates"
329+
)
330+
331+
# with newest version in place: not offering to switch
332+
withr::local_options(qgisprocess.test_try_new_qgis = "yes")
333+
saveRDS(
334+
list(
335+
path = "C:/Program Files/QGIS 3.30.0/bin/qgis_process-qgis-ltr.bat",
336+
version = "3.30.0-xxx",
337+
algorithms = qgis_algorithms(),
338+
plugins = qgis_plugins(),
339+
use_json_output = qgis_using_json_output()
340+
),
341+
cache_data_file
342+
)
343+
expect_no_message(
344+
capture.output(qgis_configure(use_cached_data = TRUE), type = "message"),
345+
message = "A newer QGIS installation seems to be"
346+
)
347+
348+
# without the option: not offering to switch
349+
withr::local_options(list(
350+
qgisprocess.detect_newer_qgis = NULL,
351+
qgisprocess.test_try_new_qgis = "yes"
352+
))
353+
saveRDS(
354+
list(
355+
path = "C:/Program Files/QGIS 3.28.6/bin/qgis_process-qgis-ltr.bat",
356+
version = "3.28.6-xxx",
357+
algorithms = qgis_algorithms(),
358+
plugins = qgis_plugins(),
359+
use_json_output = qgis_using_json_output()
360+
),
361+
cache_data_file
362+
)
363+
expect_no_message(
364+
capture.output(qgis_configure(use_cached_data = TRUE), type = "message"),
365+
message = "A newer QGIS installation seems to be"
366+
)
367+
368+
# when not interactive: not offering to switch
369+
rlang::local_interactive(value = FALSE)
370+
withr::local_options(list(
371+
qgisprocess.detect_newer_qgis = TRUE,
372+
qgisprocess.test_try_new_qgis = "yes"
373+
))
374+
saveRDS(
375+
list(
376+
path = "C:/Program Files/QGIS 3.28.6/bin/qgis_process-qgis-ltr.bat",
377+
version = "3.28.6-xxx",
378+
algorithms = qgis_algorithms(),
379+
plugins = qgis_plugins(),
380+
use_json_output = qgis_using_json_output()
381+
),
382+
cache_data_file
383+
)
384+
expect_no_message(
385+
capture.output(qgis_configure(use_cached_data = TRUE), type = "message"),
386+
message = "A newer QGIS installation seems to be"
387+
)
388+
})
389+
390+
391+
392+
265393
test_that("abort_query_version() works", {
266394
lines <- c("aa", "bb")
267395
expect_error(

‎tests/testthat/test-qgis-state.R‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,62 @@ test_that("Internal function debug_json() works", {
146146
expect_no_error(debug_json())
147147
expect_s3_class(debug_json(), "glue")
148148
})
149+
150+
151+
152+
153+
154+
155+
test_that("Internal function resolve_flag_opt() works", {
156+
expect_true(resolve_flag_opt(TRUE))
157+
expect_true(resolve_flag_opt("TRUE"))
158+
expect_true(resolve_flag_opt("true"))
159+
expect_false(resolve_flag_opt(""))
160+
expect_false(resolve_flag_opt(FALSE))
161+
expect_false(resolve_flag_opt("FALSE"))
162+
expect_false(resolve_flag_opt("false"))
163+
expect_identical(resolve_flag_opt("", keep_NA = TRUE), NA)
164+
expect_identical(resolve_flag_opt(NA, keep_NA = TRUE), NA)
165+
expect_error(resolve_flag_opt(NULL, keep_NA = TRUE), "must be")
166+
expect_error(resolve_flag_opt("maybe"), "must be")
167+
expect_error(resolve_flag_opt(c(TRUE, TRUE)), "must be")
168+
169+
expect_false(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
170+
expect_identical(
171+
resolve_flag_opt(
172+
option_name = "test_option",
173+
envvar_name = "TEST_VAR",
174+
keep_NA = TRUE
175+
),
176+
NA
177+
)
178+
179+
expect_error(resolve_flag_opt(option_name = "test_option"), "Both")
180+
expect_error(resolve_flag_opt(envvar_name = "TEST_VAR"), "Both")
181+
182+
withr::local_options(test_option = TRUE)
183+
expect_true(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
184+
expect_error(resolve_flag_opt(option_name = "test_option"), "Both")
185+
withr::local_options(test_option = "TRUE")
186+
expect_true(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
187+
withr::local_options(test_option = FALSE)
188+
expect_false(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
189+
withr::local_options(test_option = "FALSE")
190+
expect_false(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
191+
withr::local_options(test_option = 3)
192+
expect_error(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
193+
withr::local_options(test_option = NULL)
194+
expect_false(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
195+
196+
withr::local_envvar(TEST_VAR = "TRUE")
197+
expect_true(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
198+
expect_error(resolve_flag_opt(envvar_name = "TEST_VAR"), "Both")
199+
withr::local_envvar(TEST_VAR = "true")
200+
expect_true(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
201+
withr::local_envvar(TEST_VAR = "FALSE")
202+
expect_false(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
203+
withr::local_envvar(TEST_VAR = "false")
204+
expect_false(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
205+
withr::local_envvar(TEST_VAR = "3")
206+
expect_error(resolve_flag_opt(option_name = "test_option", envvar_name = "TEST_VAR"))
207+
})

0 commit comments

Comments
 (0)