From 891315d2eec5672e0ff288c925cbb151f1563e38 Mon Sep 17 00:00:00 2001 From: Johannes Rainer Date: Tue, 19 Nov 2024 15:52:35 +0100 Subject: [PATCH] Complete unit test coverage --- .editorconfig | 4 +- DESCRIPTION | 4 +- NEWS.md | 8 +- R/MsBackendSql-functions.R | 67 ++++++------ R/MsBackendSql.R | 2 +- tests/testthat.R | 7 ++ tests/testthat/test_MsBackendSql-functions.R | 103 +++++++++++++++++-- tests/testthat/test_MsBackendSql.R | 25 +++++ 8 files changed, 169 insertions(+), 51 deletions(-) diff --git a/.editorconfig b/.editorconfig index d93efa7..2243bb1 100644 --- a/.editorconfig +++ b/.editorconfig @@ -6,7 +6,7 @@ root = true charset = utf-8 end_of_line = lf trim_trailing_whitespace = true -insert_final_newline = false +insert_final_newline = true [*.R] indent_style = space @@ -22,4 +22,4 @@ indent_style = tab [.travis.yml] indent_style = space -indent_size = 2 +indent_size = 2 \ No newline at end of file diff --git a/DESCRIPTION b/DESCRIPTION index dd802c2..dc53a35 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: MsBackendSql Title: SQL-based Mass Spectrometry Data Backend -Version: 1.7.0 +Version: 1.7.1 Authors@R: c(person(given = "Johannes", family = "Rainer", email = "Johannes.Rainer@eurac.edu", @@ -54,4 +54,4 @@ RoxygenNote: 7.3.2 Collate: 'MsBackendSql-functions.R' 'MsBackendSql.R' - 'MsBackendOfflineSql.R' + 'MsBackendOfflineSql.R' \ No newline at end of file diff --git a/NEWS.md b/NEWS.md index e1e0f42..53e5cc1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,9 @@ +# MsBackendSql 1.7 + +## Changes in 1.7.1 + +- Complete unit tests to cover all code lines. + # MsBackendSql 1.5 ## Changes in 1.5.1 @@ -140,4 +146,4 @@ ## Changes in 0.0.2 -- First full implementation of `MsBackendSql`. +- First full implementation of `MsBackendSql`. \ No newline at end of file diff --git a/R/MsBackendSql-functions.R b/R/MsBackendSql-functions.R index 3b5f20e..61a7c4b 100644 --- a/R/MsBackendSql-functions.R +++ b/R/MsBackendSql-functions.R @@ -78,10 +78,10 @@ MsBackendSql <- function() { res$intensity <- NumericList(ints, compress = FALSE) } } - if (!all(columns %in% colnames(res))) - stop("Column(s) ", paste0(columns[!columns %in% names(res)], - collapse = ", "), " not available.", - call. = FALSE) + ## if (!all(columns %in% colnames(res))) + ## stop("Column(s) ", paste0(columns[!columns %in% names(res)], + ## collapse = ", "), " not available.", + ## call. = FALSE) if (any(columns == "centroided") && !is.logical(res$centroided)) res$centroided <- as.logical(res$centroided) if (any(columns == "smoothed") && !is.logical(res$smoothed)) @@ -290,8 +290,6 @@ MsBackendSql <- function() { #' #' @return `integer(1)` last used spectrum_id #' -#' @importFrom DBI dbDataType -#' #' @noRd .insert_backend <- function(con, x, index = 0L, partitionBy = "none", chunk = 0L) { @@ -395,9 +393,7 @@ MsBackendSql <- function() { sv <- spectraVariables(be) sv <- sv[!sv %in% c("mz", "intensity")] spd <- as.data.frame(spectraData(be, columns = sv)) - if (inherits(con, "MySQLConnection")) - cols <- vapply(spd, function(z) dbDataType(con, z), character(1)) - else cols <- dbDataType(con, spd) + cols <- .db_data_type(con, spd) if (blob) { .initialize_tables_blob(con, cols, partitionBy, partitionNumber) peak_table <- "msms_spectrum_peak_blob" @@ -415,13 +411,7 @@ MsBackendSql <- function() { ":elapsed"), total = length(chunks), clear = FALSE, force = TRUE) pb$tick(0) - if (.is_maria_db(con)) { - res <- dbExecute(con, "SET FOREIGN_KEY_CHECKS = 0;") - res <- dbExecute(con, "SET UNIQUE_CHECKS = 0;") - res <- dbExecute(con, "ALTER TABLE msms_spectrum DISABLE KEYS;") - res <- dbExecute(con, - paste0("ALTER TABLE ", peak_table, " DISABLE KEYS;")) - } + if (.is_maria_db(con)) res <- .disable_mysql_keys(con, peak_table) for (i in seq_along(chunks)) { s <- Spectra(source = backend, x[chunks[[i]]], BPPARAM = bpparam()) if (blob) @@ -435,6 +425,24 @@ MsBackendSql <- function() { .create_indices(con, peak_table) } +#' @importFrom DBI dbDataType +#' +#' @noRd +.db_data_type <- function(con, x) { + if (inherits(con, "MySQLConnection")) + vapply(x, function(z) dbDataType(con, z), NA_character_) + else dbDataType(con, x) +} + +.disable_mysql_keys <- function(con, peak_table = "msms_spectrum_peak") { + res <- dbExecute(con, "SET FOREIGN_KEY_CHECKS = 0;") + res <- dbExecute(con, "SET UNIQUE_CHECKS = 0;") + res <- dbExecute(con, "ALTER TABLE msms_spectrum DISABLE KEYS;") + res <- dbExecute(con, + paste0("ALTER TABLE ", peak_table, " DISABLE KEYS;")) + res +} + #' Similar to the .insert_data but takes data from the provided `Spectra` #' object and inserts that (chunk-wise) into the database. #' @@ -451,9 +459,7 @@ MsBackendSql <- function() { sv <- spectraVariables(object) sv <- sv[!sv %in% c("mz", "intensity", "spectrum_id_")] spd <- as.data.frame(spectraData(object[1], columns = sv)) - if (inherits(con, "MySQLConnection")) - cols <- vapply(spd, function(z) dbDataType(con, z), character(1)) - else cols <- dbDataType(con, spd) + cols = .db_data_type(con, spd) if (blob) { .initialize_tables_blob(con, cols, partitionBy = "none", 10) peak_table <- "msms_spectrum_peak_blob" @@ -461,13 +467,7 @@ MsBackendSql <- function() { .initialize_tables(con, cols, partitionBy = "none", 10) peak_table <- "msms_spectrum_peak" } - if (.is_maria_db(con)) { - res <- dbExecute(con, "SET FOREIGN_KEY_CHECKS = 0;") - res <- dbExecute(con, "SET UNIQUE_CHECKS = 0;") - res <- dbExecute(con, "ALTER TABLE msms_spectrum DISABLE KEYS;") - res <- dbExecute(con, - paste0("ALTER TABLE ", peak_table, " DISABLE KEYS;")) - } + if (.is_maria_db(con)) .disable_mysql_keys(con, peak_table) index <- 0 message("Importing data ... ") pb <- progress_bar$new(format = paste0("[:bar] :current/:", @@ -515,6 +515,7 @@ MsBackendSql <- function() { res <- dbExecute(con, paste0("CREATE INDEX spectrum_ms_level on ", "msms_spectrum (msLevel)")) message(" Done") + TRUE } #' @rdname MsBackendSql @@ -662,9 +663,7 @@ createMsBackendSqlDatabase <- function(dbcon, x = character(), data <- as.data.frame(data) if (nrow(data)) data$dataStorage <- "" - if (inherits(dbcon, "MySQLConnection")) - cols <- vapply(data, function(z) dbDataType(dbcon, z), character(1)) - else cols <- dbDataType(dbcon, data) + cols <- .db_data_type(dbcon, data) if (blob) { peak_table <- "msms_spectrum_peak_blob" .initialize_tables_blob(dbcon, cols) @@ -673,13 +672,7 @@ createMsBackendSqlDatabase <- function(dbcon, x = character(), .initialize_tables(dbcon, cols) } if (nrow(data)) { - if (.is_maria_db(dbcon)) { - res <- dbExecute(dbcon, "SET FOREIGN_KEY_CHECKS = 0;") - res <- dbExecute(dbcon, "SET UNIQUE_CHECKS = 0;") - res <- dbExecute(dbcon, "ALTER TABLE msms_spectrum DISABLE KEYS;") - res <- dbExecute( - dbcon, paste0("ALTER TABLE ", peak_table, " DISABLE KEYS;")) - } + if (.is_maria_db(dbcon)) .disable_mysql_keys(dbcon, peak_table) sid <- seq_len(nrow(data)) data$spectrum_id_ <- sid message("Done") @@ -717,4 +710,4 @@ createMsBackendSqlDatabase <- function(dbcon, x = character(), if (any(nas)) x[, !nas, drop = FALSE] else x -} +} \ No newline at end of file diff --git a/R/MsBackendSql.R b/R/MsBackendSql.R index 889b6fe..5121144 100644 --- a/R/MsBackendSql.R +++ b/R/MsBackendSql.R @@ -551,7 +551,7 @@ setMethod( pks <- object@peak_fun(object, columns) if (is.list(pks$mz) | is.list(pks$intensity)) { res <- do.call( - mapply, args = c(pks[columns], + base::mapply, args = c(pks[columns], list(FUN = base::cbind, SIMPLIFY = FALSE, USE.NAMES = FALSE))) res[match(object@spectraIds, pks$spectrum_id_)] diff --git a/tests/testthat.R b/tests/testthat.R index d28c74a..63038ab 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -4,6 +4,13 @@ library(Spectra) library(RSQLite) library(msdata) +setClass("DummySQL", + contains = "SQLiteConnection") + +setMethod("dbExecute", c("DummySQL", "character"), function(conn, statement) { + TRUE +}) + mm8_file <- system.file("microtofq", "MM8.mzML", package = "msdata") mm8_sps <- Spectra(mm8_file) mm8_db <- dbConnect(SQLite(), tempfile()) diff --git a/tests/testthat/test_MsBackendSql-functions.R b/tests/testthat/test_MsBackendSql-functions.R index ebf8fed..3d256e3 100644 --- a/tests/testthat/test_MsBackendSql-functions.R +++ b/tests/testthat/test_MsBackendSql-functions.R @@ -33,11 +33,21 @@ test_that(".insert_data et al work", { names(pks) <- NULL expect_equal(as.list(peaksData(mm8_sps)), pks) dbDisconnect(db) + + ## Mocking a MySQL connection + db_file <- tempfile() + db <- dbConnect(SQLite(), db_file) + db <- as(db, "DummySQL") + with_mocked_bindings( + ".is_maria_db" = function(x) TRUE, + code = .insert_data(db, mm8_file) + ) + dbDisconnect(db) }) test_that(".set_backend_insert_data works", { s <- Spectra(c(mm8_file, mm14_file)) - expect_error(.set_backend_insert_data(object, f = c(1, 2, 3))) + expect_error(.set_backend_insert_data(s, f = c(1, 2, 3)), "match length") con_ref <- dbConnect(SQLite(), tempfile()) createMsBackendSqlDatabase(con_ref, c(mm8_file, mm14_file)) @@ -81,6 +91,17 @@ test_that(".set_backend_insert_data works", { expect_equal(dbGetQuery(con_ref, "select * from msms_spectrum_peak_blob"), dbGetQuery(con_test, "select * from msms_spectrum_peak_blob")) + ## mock a MySQL connection + dbDisconnect(con_test) + con_test <- dbConnect(SQLite(), tempfile()) + con_test <- as(con_test, "DummySQL") + with_mocked_bindings( + ".insert_backend_blob" = function(...) TRUE, + ".is_maria_db" = function(x) TRUE, + code = expect_true(.set_backend_insert_data(s, f = factor(), + con = con_test)) + ) + dbDisconnect(con_ref) dbDisconnect(con_test) }) @@ -136,7 +157,15 @@ test_that(".fetch_spectra_data_sql works", { expect_identical(length(mm8_be), nrow(res)) }) +test_that(".disable_mysql_keys works", { + ## Mocking the call since we don't have a MySQL database connection for + ## testing + expect_true(.disable_mysql_keys(new("DummySQL"))) +}) + test_that(".spectra_data_sql works", { + expect_error(.spectra_data_sql(mm8_be, c("rtime", "other_col")), + "other_col not available.") res <- .spectra_data_sql(mm8_be, c("rtime", "msLevel", "mz")) expect_s4_class(res, "DataFrame") expect_identical(colnames(res), c("rtime", "msLevel", "mz")) @@ -174,6 +203,15 @@ test_that(".spectra_data_sql works", { expect_equal(tmp_sps$intensity, tmp$intensity) }) +test_that(".db_data_type works", { + x <- data.frame(a = 1:4, b = TRUE, c = "TRUE") + res <- .db_data_type(new("SQLiteConnection"), x) + expect_equal(res, c(a = "INT", b = "SMALLINT", c = "TEXT")) + setClass("MySQLConnection", contains = "SQLiteConnection") + res <- .db_data_type(new("MySQLConnection"), x) + expect_equal(res, c(a = "INTEGER", b = "INTEGER", c = "TEXT")) +}) + test_that(".available_peaks_variables works", { res <- .available_peaks_variables(mm8_be) expect_equal(res, c("mz", "intensity")) @@ -228,6 +266,25 @@ test_that(".combine works", { res <- .combine(tmp) expect_s4_class(res, "MsBackendSql") expect_equal(mm8_be[1:10], res) + + tmp <- .combine(list(mm8_be)) + expect_equal(length(tmp), length(mm8_be)) + expect_equal(rtime(tmp), rtime(mm8_be)) + + a <- mm8_be[1:10] + b <- setBackend(Spectra(a), MsBackendMemory())@backend + expect_error(.combine(list(a, b)), "Can only merge backends of the same") + + expect_error(.combine(list(mm8_be, mm_be)), "connected to the same") +}) + +test_that(".initialize_tables works", { + a <- new("DummySQL") + MsBackendSql:::.initialize_tables(a, cols = c(a = "TEXT")) + with_mock( + "MsBackendSql:::.is_maria_db" = function(x) TRUE, + .initialize_tables(a, cols = c(a = "TEXT")) + ) }) test_that(".initialize_tables_sql works", { @@ -349,6 +406,20 @@ test_that(".create_from_spectra_data works", { tbls <- dbListTables(tmpcon) expect_equal(tbls, c("msms_spectrum", "msms_spectrum_peak_blob")) + expect_error(.create_from_spectra_data(tmpcon, dta), + "contains already tables of a") + + dbDisconnect(tmpcon) + + tmpf <- tempfile() + tmpcon <- dbConnect(SQLite(), tmpf) + dta_2 <- dta[, !colnames(dta) %in% c("rtime", "msLevel")] + .create_from_spectra_data(tmpcon, dta_2, blob = FALSE) + res2 <- backendInitialize(MsBackendSql(), dbcon = tmpcon) + expect_true(all(is.na(res2$msLevel))) + expect_true(all(is.na(res2$rtime))) + dbDisconnect(tmpcon) + ## long format tmpf <- tempfile() tmpcon <- dbConnect(SQLite(), tmpf) @@ -356,10 +427,11 @@ test_that(".create_from_spectra_data works", { tbls <- dbListTables(tmpcon) expect_equal(tbls, c("msms_spectrum", "msms_spectrum_peak")) res2 <- backendInitialize(MsBackendSql(), dbcon = tmpcon) - expect_true(all(res2$dataStorage != res$dataStorage)) - expect_equal(rtime(res2), rtime(res)) - expect_equal(mz(res2), mz(res)) - expect_equal(intensity(res2), intensity(res)) + expect_true(all(res2$dataStorage != mm8_be$dataStorage)) + expect_equal(rtime(res2), rtime(mm8_be)) + expect_equal(mz(res2), mz(mm8_be)) + expect_equal(intensity(res2), intensity(mm8_be)) + dbDisconnect(tmpcon) ## empty data frame tmpf <- tempfile() @@ -370,8 +442,8 @@ test_that(".create_from_spectra_data works", { .create_from_spectra_data(tmpcon, dta) res3 <- backendInitialize(MsBackendSql(), dbcon = tmpcon) expect_true(validObject(res3)) - expect_equal(spectraVariables(res2), spectraVariables(res3)) - expect_equal(colnames(spectraData(res2)), colnames(spectraData(res3))) + expect_equal(spectraVariables(mm8_be), spectraVariables(res3)) + expect_equal(colnames(spectraData(mm8_be)), colnames(spectraData(res3))) expect_true(length(res3) == 0L) dbDisconnect(tmpcon) @@ -393,6 +465,21 @@ test_that(".create_from_spectra_data works", { compress = FALSE)) expect_equal(tmp$msLevel, dta$msLevel) expect_equal(tmp$rtime, dta$rtime) + dbDisconnect(tmpcon) + + tmpf <- tempfile() + tmpcon <- dbConnect(SQLite(), tmpf) + tmpcon <- as(tmpcon, "DummySQL") + dta <- spectraData( + mm8_sps, columns = c(spectraVariables(mm8_sps), "mz", "intensity")) + ## With mock to simulate MariaDB + with_mocked_bindings( + ".insert_spectra_variables" = function(...) TRUE, + ".insert_peaks" = function(...) TRUE, + ".is_maria_db" = function(x) TRUE, + code = .create_from_spectra_data(tmpcon, dta, blob = FALSE) + ) + dbDisconnect(tmpcon) }) test_that(".drop_na_columns works", { @@ -420,4 +507,4 @@ test_that(".drop_na_columns works", { tmp$d <- 9 res <- .drop_na_columns(tmp) expect_equal(res, tmp) -}) +}) \ No newline at end of file diff --git a/tests/testthat/test_MsBackendSql.R b/tests/testthat/test_MsBackendSql.R index 69e4ce8..3724279 100644 --- a/tests/testthat/test_MsBackendSql.R +++ b/tests/testthat/test_MsBackendSql.R @@ -76,6 +76,14 @@ test_that("[,MsBackendSql works", { expect_true(length(res) == 1L) expect_true(is.matrix(res[[1L]])) expect_equal(res[[1L]], peaksData(mm8_sps[1L])[[1L]]) + + tmp <- mm8_be + tmp@spectraIds <- c(tmp@spectraIds, 300L) + res <- peaksData(tmp) + res <- res[[length(res)]] + expect_true(is.matrix(res)) + expect_true(nrow(res) == 0) + }) test_that("peaksData,MsBackendSql works", { @@ -191,6 +199,9 @@ test_that("filterMsLevel,MsBackendSql works", { res <- filterMsLevel(mm8_be) expect_equal(res, mm8_be) + res <- filterMsLevel(mm8_be, msLevel = integer()) + expect_equal(res, mm8_be) + res <- filterMsLevel(mm8_be, msLevel = 1:2) expect_equal(res, mm8_be) @@ -246,6 +257,9 @@ test_that("filterRt,MsBackendSql works", { }) test_that("filterDataOrigin works", { + res <- filterDataOrigin(mm_be, character()) + expect_equal(res, mm_be) + res <- filterDataOrigin(mm_be, normalizePath(mm8_file)) expect_true(all(res$dataOrigin == normalizePath(mm8_file))) @@ -254,6 +268,14 @@ test_that("filterDataOrigin works", { res <- filterDataOrigin(mm_be, normalizePath(c(mm14_file, mm8_file))) expect_equal(unique(dataOrigin(res)), normalizePath(c(mm14_file, mm8_file))) + + tmp <- mm_be + tmp@localData$dataOrigin <- "here" + tmp@localData$dataOrigin[1:20] <- "there" + res <- filterDataOrigin(tmp, "here") + expect_true(all(res$dataOrigin == "here")) + expect_equal(res@spectraIds, mm_be@spectraIds[21:310]) + expect_equal(rtime(res), rtime(mm_be[21:310])) }) test_that("filterPrecursorMzRange works", { @@ -268,6 +290,9 @@ test_that("filterPrecursorMzRange works", { }) test_that("filterPrecursorMzValues works", { + res <- filterPrecursorMzValues(tmt_be, mz = numeric()) + expect_equal(res, tmt_be) + tmt_be2 <- tmt_be tmt_be2$precursorMz <- tmt_be$precursorMz pmz <- c(620.1, 404.25, 417.7, 506.6)