From bcab9bf68c874be6d1014bbd26cae6b0333caf3c Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Tue, 29 Sep 2026 17:17:44 -0400 Subject: [PATCH 1/2] fix(pkg-r): skip tests that assume Suggests packages are installed CRAN's r-devel-linux-x86_64-fedora-clang flavor has RSQLite but not duckdb installed, which caused the test suite to ERROR: - 92 errors: tests requesting engine = "duckdb" (explicitly or via the local_data_frame_source() fixture default) hard-failed in check_installed("duckdb"), including one variant where the non-interactive check surfaced as "menu() cannot be used non-interactively" - 4 failures: tests using the default engine silently fell back to SQLite and then failed DuckDB-specific expectations skip_if_no_dataframe_engine() only skipped when *neither* engine was installed, so it never fired on that machine, and it had no way to express "this test requires a specific engine". It is now engine-aware: - with engine = NULL, keep the old any-engine semantics (mirroring how DataFrameSource resolves a default engine) - with a specific engine, skip unless that engine's package is installed, so tests no longer silently degrade to another engine local_data_frame_source() and local_recording_data_frame_source() now call it with their requested engine, and local_querychat() skips for plain data.frame inputs, so fixture-based tests skip automatically. Tests that construct DataFrameSource or QueryChat directly gained skip_if_no_dataframe_engine() guards (6 describe blocks and 3 test_that() blocks in test-QueryChat.R, 2 describe blocks in test-QueryChatSystemPrompt.R, plus 3 explicit skip_if_not_installed ("duckdb") calls in DuckDB-asserting tests). Audited all Suggests packages by running the full suite with every Suggests dependency (except the testthat/withr harness and later, which is a hard dependency of shiny) shadowed by broken stubs. That surfaced 46 additional tests failing on "no compatible database engine" with both engines absent, all fixed by the guards above. Verified by simulating the CRAN condition (FAIL 0, SKIP 98, PASS 716 for the duckdb-only case), a full Suggests blackout (FAIL 0, SKIP 199, PASS 1198), and with everything installed (FAIL 0, SKIP 0, PASS 2195, twice). --- pkg-r/tests/testthat/helper-fixtures.R | 37 +++++++++++++++++-- pkg-r/tests/testthat/test-QueryChat.R | 17 +++++++++ .../testthat/test-QueryChatSystemPrompt.R | 10 +++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/pkg-r/tests/testthat/helper-fixtures.R b/pkg-r/tests/testthat/helper-fixtures.R index 9b254fa07..7f49235f3 100644 --- a/pkg-r/tests/testthat/helper-fixtures.R +++ b/pkg-r/tests/testthat/helper-fixtures.R @@ -76,11 +76,33 @@ local_sqlite_connection <- function( list(conn = conn, path = temp_db) } -# Skip test if no DataFrameSource engine is available -skip_if_no_dataframe_engine <- function() { - if (!rlang::is_installed("duckdb") && !rlang::is_installed("RSQLite")) { - skip("Neither duckdb nor RSQLite is installed") +# Skip test if no DataFrameSource engine is available. +# +# When `engine` is NULL, skip only if neither duckdb nor RSQLite is installed +# (mirroring how DataFrameSource resolves a default engine). When a specific +# engine is requested, skip unless that engine's package is installed, so +# tests never silently fall back to a different engine (e.g. SQLite) and then +# fail their DuckDB-specific expectations. +skip_if_no_dataframe_engine <- function(engine = NULL) { + if (is.null(engine)) { + engine <- getOption("querychat.DataFrameSource.engine", NULL) } + + if (is.null(engine)) { + if (!rlang::is_installed("duckdb") && !rlang::is_installed("RSQLite")) { + skip("Neither duckdb nor RSQLite is installed") + } + return(invisible()) + } + + engine <- tolower(engine) + if (engine == "duckdb") { + skip_if_not_installed("duckdb") + } else if (engine == "sqlite") { + skip_if_not_installed("RSQLite") + } + + invisible() } # Create a DataFrameSource with automatic cleanup @@ -90,6 +112,7 @@ local_data_frame_source <- function( engine = "duckdb", env = parent.frame() ) { + skip_if_no_dataframe_engine(engine) df_source <- DataFrameSource$new(data, table_name, engine = engine) withr::defer(df_source$cleanup(), envir = env) df_source @@ -119,6 +142,7 @@ local_recording_data_frame_source <- function( engine = "duckdb", env = parent.frame() ) { + skip_if_no_dataframe_engine(engine) state <- new.env(parent = emptyenv()) state$get_data_calls <- 0L state$get_data_error <- NULL @@ -339,6 +363,11 @@ local_querychat <- function( ..., env = parent.frame() ) { + # Plain data frames are wrapped in a DataFrameSource using the default + # engine, which requires duckdb or RSQLite to be installed. + if (is.data.frame(data_source)) { + skip_if_no_dataframe_engine() + } qc <- QueryChat$new(data_source, table_name, ...) withr::defer(qc$cleanup(), envir = env) qc diff --git a/pkg-r/tests/testthat/test-QueryChat.R b/pkg-r/tests/testthat/test-QueryChat.R index f3a4720ae..2499c17af 100644 --- a/pkg-r/tests/testthat/test-QueryChat.R +++ b/pkg-r/tests/testthat/test-QueryChat.R @@ -222,6 +222,8 @@ describe("QueryChat integration with DBISource", { }) describe("QueryChat$cleanup()", { + skip_if_no_dataframe_engine() + it("cleans up data source resources", { test_df <- new_test_df() qc <- QueryChat$new(test_df, greeting = "Test") @@ -235,6 +237,8 @@ describe("QueryChat$cleanup()", { }) describe("QueryChat$system_prompt", { + skip_if_no_dataframe_engine() + it("returns the system prompt from the client", { test_df <- new_test_df() qc <- QueryChat$new(test_df, greeting = "Test") @@ -328,6 +332,8 @@ describe("QueryChat$data_source", { }) describe("QueryChat$client()", { + skip_if_no_dataframe_engine() + it("uses default tools when tools = NA", { qc <- QueryChat$new( new_test_df(), @@ -678,6 +684,8 @@ describe("QueryChat$client()", { }) test_that("QueryChat$generate_greeting() generates a greeting using the LLM client", { + skip_if_no_dataframe_engine() + client <- mock_ellmer_chat_client( public = list( chat = function(message, ...) { @@ -698,6 +706,7 @@ test_that("QueryChat$generate_greeting() generates a greeting using the LLM clie }) test_that("QueryChat$server() errors when called outside Shiny context", { + skip_if_no_dataframe_engine() withr::local_envvar(OPENAI_API_KEY = "boop") test_df <- new_test_df() @@ -710,6 +719,8 @@ test_that("QueryChat$server() errors when called outside Shiny context", { }) test_that("QueryChat$new() validates history and stores it verbatim", { + skip_if_no_dataframe_engine() + test_df <- new_test_df() qc_default <- QueryChat$new(test_df, greeting = "Test") @@ -835,6 +846,8 @@ test_that("QueryChat$app_obj() infers Shiny bookmarking from history's restore_m }) describe("QueryChat internal client handoff availability", { + skip_if_no_dataframe_engine() + local_mocked_r6_class( QueryChat, public = list( @@ -914,6 +927,8 @@ describe("querychat()", { }) describe("QueryChat$console()", { + skip_if_no_dataframe_engine() + local_mocked_r6_class( QueryChat, public = list( @@ -1086,6 +1101,8 @@ test_that("querychat_app() only cleans up data frame sources on exit", { }) describe("QueryChat$server() client override", { + skip_if_no_dataframe_engine() + it("accepts a client parameter", { withr::local_envvar(OPENAI_API_KEY = "boop") test_df <- new_test_df() diff --git a/pkg-r/tests/testthat/test-QueryChatSystemPrompt.R b/pkg-r/tests/testthat/test-QueryChatSystemPrompt.R index 1138311e7..1954aea09 100644 --- a/pkg-r/tests/testthat/test-QueryChatSystemPrompt.R +++ b/pkg-r/tests/testthat/test-QueryChatSystemPrompt.R @@ -125,6 +125,8 @@ describe("QueryChatSystemPrompt$new()", { }) describe("QueryChatSystemPrompt$render()", { + skip_if_no_dataframe_engine() + it("renders handoff guidance only when available", { df <- new_test_df() ds <- DataFrameSource$new(df, "test_table") @@ -268,6 +270,8 @@ describe("QueryChatSystemPrompt$render()", { }) it("includes db_type in rendered output", { + skip_if_not_installed("duckdb") + df <- new_test_df() ds <- DataFrameSource$new(df, "test_table") withr::defer(ds$cleanup()) @@ -370,6 +374,8 @@ describe("QueryChatSystemPrompt$render()", { }) it("detects DuckDB correctly", { + skip_if_not_installed("duckdb") + df <- new_test_df() ds <- DataFrameSource$new(df, "test_table") withr::defer(ds$cleanup()) @@ -405,6 +411,8 @@ describe("QueryChatSystemPrompt$render()", { }) describe("QueryChatSystemPrompt with full prompt.md template", { + skip_if_no_dataframe_engine() + it("renders full template with data_description", { df <- new_test_df(3) ds <- DataFrameSource$new(df, "test_table") @@ -428,6 +436,8 @@ describe("QueryChatSystemPrompt with full prompt.md template", { }) it("includes DuckDB-specific content for DuckDB sources", { + skip_if_not_installed("duckdb") + df <- new_test_df() ds <- DataFrameSource$new(df, "test_table") withr::defer(ds$cleanup()) From 7a56f67bfe905e8aa51f83ae2006549837b64543 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Tue, 29 Sep 2026 17:36:26 -0400 Subject: [PATCH 2/2] docs(pkg-r): add NEWS bullet for CRAN test skip fix --- pkg-r/NEWS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg-r/NEWS.md b/pkg-r/NEWS.md index 4f27669ba..520a87325 100644 --- a/pkg-r/NEWS.md +++ b/pkg-r/NEWS.md @@ -1,5 +1,9 @@ # querychat (development version) +## Bug fixes + +* Tests no longer fail on systems where Suggests packages like duckdb or RSQLite aren't installed (as on some CRAN check flavors): test fixtures now skip when a required database engine is missing instead of erroring or silently falling back to a different engine. (#317) + # querychat 0.4.0 ## New features