Complete unit test coverage

.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

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 = "[email protected]",
@@ -54,4 +54,4 @@ RoxygenNote: 7.3.2
 Collate:
     'MsBackendSql-functions.R'
     'MsBackendSql.R'
-    'MsBackendOfflineSql.R'
+    'MsBackendOfflineSql.R'

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`.

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,23 +459,15 @@ 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"
     } else {
         .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 <- "<database>"
-    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
-}
+}

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_)]

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())

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,17 +406,32 @@ 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)
     .create_from_spectra_data(tmpcon, dta, blob = FALSE)
     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)
-})
+})