diff --git a/.Rbuildignore b/.Rbuildignore index 5b472194..e5e647a4 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -33,3 +33,6 @@ ^.a5c$ ^README\.html$ ^tools$ +^tests/testthat/fixtures/tiny-hub-groot/\.db\.cache$ +^tests/testthat/fixtures/tiny-hub-groot/\.ro_attributes$ +^\.worktrees$ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e3553a59..2f127618 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,6 +40,20 @@ Sys.setenv(TESTTHAT_PARALLEL = "TRUE") devtools::test() ``` +A parallel run gives each test file its own copy of the test database under +`tempdir()`, so point `TMPDIR` at a filesystem with several GB free before +starting R: + +```sh +TMPDIR=/path/with/space R -e 'Sys.setenv(TESTTHAT_PARALLEL="TRUE"); devtools::test()' +``` + +On a shared machine `/tmp` often sits on a full root filesystem. Running out +of space mid-run leaves partially copied test databases, and the resulting +failures point at whichever test file happened to use them rather than at the +disk — expect errors like `Interval test.fixedbin does not exist`. Failures +that move between files across runs are a symptom of this, not of flaky tests. + GitHub Actions runs `R CMD check` on every push and PR — treat that as the authoritative signal. diff --git a/tests/testthat/helper-liftover.R b/tests/testthat/helper-liftover.R index 71664e5b..471c12d0 100644 --- a/tests/testthat/helper-liftover.R +++ b/tests/testthat/helper-liftover.R @@ -108,6 +108,9 @@ setup_db <- function(chrom_defs, return_db = FALSE) { { unlink(db, recursive = TRUE) unlink(fastas) + # Do not leave GROOT pointing at the directory we just removed: + # the next file in this parallel worker would inherit it. + ensure_valid_groot() }, testthat::teardown_env() ) diff --git a/tests/testthat/helper-test_db.R b/tests/testthat/helper-test_db.R index 436fb7d6..122ad951 100644 --- a/tests/testthat/helper-test_db.R +++ b/tests/testthat/helper-test_db.R @@ -99,30 +99,82 @@ local_db_state <- function(env = parent.frame()) { #' This approach provides complete isolation between parallel test processes #' while minimizing disk space and setup time. #' -#' @return Path to the isolated test database -create_isolated_test_db <- function() { - source_db <- if (getOption("gmulticontig.indexed_format", FALSE)) { +#' Path to the shared, read-only test database +shared_test_db_path <- function() { + if (getOption("gmulticontig.indexed_format", FALSE)) { "/net/mraid20/ifs/wisdom/tanay_lab/tgdata/db/tgdb/misha_test_db_indexed/" } else { "/net/mraid20/export/tgdata/db/tgdb/misha_test_db/" } +} + +#' Guarantee this test file leaves a usable GROOT behind +#' +#' Files that build their own temporary database and re-root into it leave +#' .misha$GROOT dangling as soon as that directory is removed (withr deletes +#' it at the end of the test or the file). Under TESTTHAT_PARALLEL the next +#' file in the same worker process inherits the dangling root and fails with +#' something unrelated to its own subject matter - "Database directory does +#' not exist", "Chromosome chr1 does not exist ... Known chromosomes: chrA", +#' or "Cannot delete track from read-only database" (a deleted directory is +#' not writable, so it reads as read-only). +#' +#' Call this once at the top of any file that re-roots. It is idempotent and +#' does nothing when the file leaves a valid root behind. +ensure_valid_groot <- function() { + groot <- if (exists("GROOT", envir = .misha, inherits = FALSE)) { + get("GROOT", envir = .misha) + } else { + NULL + } + if (is.null(groot) || !dir.exists(groot)) { + src <- shared_test_db_path() + if (dir.exists(src)) { + suppressMessages(gdb.init(src)) + } + } + invisible(NULL) +} + +restore_groot_on_exit <- function(envir = parent.frame()) { + withr::defer(ensure_valid_groot(), envir = envir) +} + +#' @return Path to the isolated test database +create_isolated_test_db <- function() { + source_db <- shared_test_db_path() + + # Fail loudly instead of leaving a half-built DB behind. A silent failure + # here (a full tempdir() is the usual cause) surfaces much later as a + # baffling "Interval test.fixedbin does not exist" in an unrelated test. + checked_system <- function(cmd) { + status <- system(cmd) + if (status != 0) { + stop(sprintf( + "create_isolated_test_db(): `%s` failed with status %d.\nIs tempdir() (%s) out of space? Set TMPDIR to a volume with room.", + cmd, status, tempdir() + ), call. = FALSE) + } + } # Create unique temp dir for this test file/process testdb_dir <- tempfile(pattern = "misha_testdb_", tmpdir = tempdir()) dir.create(testdb_dir, showWarnings = FALSE) # Copy small files that might be read - file.copy( - file.path(source_db, "chrom_sizes.txt"), - testdb_dir - ) + if (!file.copy(file.path(source_db, "chrom_sizes.txt"), testdb_dir)) { + stop(sprintf( + "create_isolated_test_db(): failed to copy chrom_sizes.txt into %s (out of space?)", + testdb_dir + ), call. = FALSE) + } # Copy interval sets and PSSM directories - system(sprintf("cp -r %s/intervs %s/", source_db, testdb_dir)) - system(sprintf("cp -r %s/pssms %s/", source_db, testdb_dir)) + checked_system(sprintf("cp -r %s/intervs %s/", source_db, testdb_dir)) + checked_system(sprintf("cp -r %s/pssms %s/", source_db, testdb_dir)) # Symlink the large seq directory (read-only) - system(sprintf("ln -s %s/seq %s/seq", source_db, testdb_dir)) + checked_system(sprintf("ln -s %s/seq %s/seq", source_db, testdb_dir)) # Create tracks directory for this test file tracks_dir <- file.path(testdb_dir, "tracks") @@ -137,7 +189,7 @@ create_isolated_test_db <- function() { # Symlink each fixture track for (track in source_tracks) { - system(sprintf( + checked_system(sprintf( "ln -s %s/tracks/%s %s/tracks/%s", source_db, track, testdb_dir, track )) @@ -170,6 +222,14 @@ create_isolated_test_db <- function() { if (!is.null(current_groot) && identical(current_groot, testdb_dir)) { if (!is.null(prev_groot) && dir.exists(prev_groot)) { suppressMessages(gdb.init(prev_groot)) + } else if (dir.exists(source_db)) { + # prev_groot was itself an isolated db that has already been + # torn down. Fall back to the shared test db rather than + # unsetting GROOT: in a parallel run the next file in this + # worker may be one that relies on the ambient root, and it + # would otherwise fail with a confusing "does not exist" or + # "Chromosome chr1 does not exist" error. + suppressMessages(gdb.init(source_db)) } else { rm("GROOT", envir = .misha) } diff --git a/tests/testthat/test-bigset-character-chrom.R b/tests/testthat/test-bigset-character-chrom.R index b94f967f..c1178d1d 100644 --- a/tests/testthat/test-bigset-character-chrom.R +++ b/tests/testthat/test-bigset-character-chrom.R @@ -4,6 +4,8 @@ # files store $chrom as factor while the persisted .meta zeroline kept the # original character class. +restore_groot_on_exit() + test_that("bigset save then load works when input has character chrom", { skip_if_not_installed("withr") withr::local_options(list(gmulticontig.indexed_format = FALSE)) diff --git a/tests/testthat/test-bigset-fast-load.R b/tests/testthat/test-bigset-fast-load.R index d3421094..6522ecde 100644 --- a/tests/testthat/test-bigset-fast-load.R +++ b/tests/testthat/test-bigset-fast-load.R @@ -1,5 +1,7 @@ # Tests for fast-path loading of indexed bigsets +restore_groot_on_exit() + test_that(".gintervals.is_indexed_bigset detects indexed bigsets correctly", { skip_if_not_installed("withr") withr::local_options(list(gmulticontig.indexed_format = FALSE)) diff --git a/tests/testthat/test-db-format-conversion.R b/tests/testthat/test-db-format-conversion.R index 524b7e91..fb64848b 100644 --- a/tests/testthat/test-db-format-conversion.R +++ b/tests/testthat/test-db-format-conversion.R @@ -1,3 +1,5 @@ +restore_groot_on_exit() + load_test_db() test_that("gdb.convert_to_indexed converts per-chromosome database to indexed format", { local_db_state() diff --git a/tests/testthat/test-gdb-convert-parallel.R b/tests/testthat/test-gdb-convert-parallel.R index 40e45de8..64644b92 100644 --- a/tests/testthat/test-gdb-convert-parallel.R +++ b/tests/testthat/test-gdb-convert-parallel.R @@ -1,3 +1,5 @@ +restore_groot_on_exit() + load_test_db() # Tests for parallel gdb.convert_to_indexed (threads argument). Each test diff --git a/tests/testthat/test-ggenome.R b/tests/testthat/test-ggenome.R index b49e1676..52121f4c 100644 --- a/tests/testthat/test-ggenome.R +++ b/tests/testthat/test-ggenome.R @@ -1,3 +1,5 @@ +create_isolated_test_db() + # Local test helper: read a FASTA file into a named list of sequences read_fasta <- function(fasta_path) { lines <- readLines(fasta_path) diff --git a/tests/testthat/test-gintervals-2d-meta-sparse.R b/tests/testthat/test-gintervals-2d-meta-sparse.R index b11112f7..78119f6d 100644 --- a/tests/testthat/test-gintervals-2d-meta-sparse.R +++ b/tests/testthat/test-gintervals-2d-meta-sparse.R @@ -4,6 +4,8 @@ # load + query path here with a moderate-sized N (1000 contigs) and only a # handful of populated chrom-pairs. +restore_groot_on_exit() + build_many_contig_db <- function(n_contigs, defer_envir) { skip_if_not_installed("withr") tmp_root <- withr::local_tempdir(.local_envir = defer_envir) diff --git a/tests/testthat/test-gintervals-from-strings.R b/tests/testthat/test-gintervals-from-strings.R index 584b1c3f..16fe3665 100644 --- a/tests/testthat/test-gintervals-from-strings.R +++ b/tests/testthat/test-gintervals-from-strings.R @@ -1,3 +1,5 @@ +create_isolated_test_db() + test_that("gintervals.from_strings parses a single chrom:start-end string", { res <- gintervals.from_strings("chr1:100-200") expect_equal(nrow(res), 1) diff --git a/tests/testthat/test-gtrack-create-meta-indexed.R b/tests/testthat/test-gtrack-create-meta-indexed.R index e3a2cd6e..b4c0788f 100644 --- a/tests/testthat/test-gtrack-create-meta-indexed.R +++ b/tests/testthat/test-gtrack-create-meta-indexed.R @@ -13,6 +13,8 @@ # 2. Meta creation completes for an indexed track where the overwhelming # majority of chromids have no data (the large-contig case). +restore_groot_on_exit() + setup_db <- function(num_chroms, chrom_size = 1e6, indexed = FALSE) { tmp_root <- withr::local_tempdir(.local_envir = parent.frame()) chrom_sizes <- data.frame( diff --git a/tests/testthat/test-index-cache-invalidation.R b/tests/testthat/test-index-cache-invalidation.R index e5223a90..4bf3e739 100644 --- a/tests/testthat/test-index-cache-invalidation.R +++ b/tests/testthat/test-index-cache-invalidation.R @@ -16,6 +16,8 @@ # These tests exercise the rm-then-recreate cycle and verify the next # read succeeds. +restore_groot_on_exit() + # Build a tiny per-chromosome DB then convert it to indexed format. build_indexed_test_db <- function() { test_db <- tempfile("misha_idxcache_") diff --git a/tests/testthat/test-indexed-integration.R b/tests/testthat/test-indexed-integration.R index 95190da6..a6fb03ab 100644 --- a/tests/testthat/test-indexed-integration.R +++ b/tests/testthat/test-indexed-integration.R @@ -1,3 +1,5 @@ +restore_groot_on_exit() + load_test_db() # Integration tests for indexed genome format diff --git a/tests/testthat/test-track-indexed-direct.R b/tests/testthat/test-track-indexed-direct.R index b4633988..1d31bf8d 100644 --- a/tests/testthat/test-track-indexed-direct.R +++ b/tests/testthat/test-track-indexed-direct.R @@ -4,6 +4,8 @@ # Build a small indexed DB from a 5-chromosome FASTA and create the # named track via gtrack.create. Returns the path to the track dir. +restore_groot_on_exit() + .make_indexed_db_with_track <- function(track_name, bin_size = 4) { test_fasta <- tempfile(fileext = ".fasta") cat(">chr1\nACTGACTGACTGACTGACTGACTGACTGACTG\n",