Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update tests + various lints #922

Merged
merged 12 commits into from
Jan 31, 2024
2 changes: 1 addition & 1 deletion R/class-workbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -4784,7 +4784,7 @@ wbWorkbook <- R6::R6Class(
pos <- '<xdr:pos x="0" y="0" />'

drawingsXML <- stri_join(
'<xdr:absoluteAnchor>',
"<xdr:absoluteAnchor>",
pos,
sprintf('<xdr:ext cx="%s" cy="%s"/>', width, height),
genBasePic(imageNo, next_id),
Expand Down
4 changes: 2 additions & 2 deletions R/class-worksheet.R
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ wbWorksheet <- R6::R6Class(

if (!is.null(tab_color)) {
tab_color <- xml_node_create("tabColor", xml_attributes = tab_color)
tabColor <- sprintf('<sheetPr>%s</sheetPr>', tab_color)
tabColor <- sprintf("<sheetPr>%s</sheetPr>", tab_color)
} else {
tabColor <- character()
}
Expand Down Expand Up @@ -280,7 +280,7 @@ wbWorksheet <- R6::R6Class(
if (length(self$cols_attr)) {
paste(c("<cols>", self$cols_attr, "</cols>"), collapse = "")
},
'</worksheet>',
"</worksheet>",
sep = ""
)
},
Expand Down
2 changes: 1 addition & 1 deletion R/helper-functions.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ create_hyperlink <- function(sheet, row = 1, col = 1, text = NULL, file = NULL)
}

if (is.null(text)) {
str <- sprintf('=HYPERLINK(%s)', dest)
str <- sprintf("=HYPERLINK(%s)", dest)
} else {
str <- sprintf('=HYPERLINK(%s, \"%s\")', dest, text)
}
Expand Down
4 changes: 2 additions & 2 deletions R/read.R
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,8 @@ wb_to_df <- function(
cc_tab <- unique(cc$c_t)

# bool
if (any(cc_tab == c("b"))) {
sel <- cc$c_t %in% c("b")
if (any(cc_tab == "b")) {
sel <- cc$c_t %in% "b"
cc$val[sel] <- as.logical(as.numeric(cc$v[sel]))
cc$typ[sel] <- "b"
}
Expand Down
1 change: 1 addition & 0 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,7 @@ read_xml_files <- function(x) {
#' @noRd
un_list <- function(x) {

# TODO can use `lengths()` when depending on R 4.0
names <- vapply(x, length, NA_integer_)
nams <- NULL
for (i in seq_along(names)) {
Expand Down
15 changes: 7 additions & 8 deletions tests/testthat/test-class-comment.R
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,9 @@ test_that("print comment", {

c2 <- create_comment(text = "this is another comment",
author = "Marco Polo")

exp <- "Author: Marco Polo\nText:\n Marco Polo:\nthis is another comment\n\nStyle:\n\n\n\n\nFont name: Aptos Narrow\nFont size: 11\nFont color: #000000\n\n"
got <- capture_output(print(c2), print = TRUE)
expect_equal(exp, got)
exp <- "Author: Marco Polo\nText:\n Marco Polo:\nthis is another comment\n\nStyle:\n\n\n\n\nFont name: Aptos Narrow\nFont size: 11\nFont color: #000000\n\n"
expect_equal(got, exp)
JanMarvin marked this conversation as resolved.
Show resolved Hide resolved

})

Expand Down Expand Up @@ -330,11 +329,11 @@ test_that("background images work", {
wb$add_comment(dims = "G12", comment = c1, file = tmp2)
wb$add_comment(dims = "G12", sheet = 1, comment = c1, file = tmp2)

expect_equal(3, length(wb$vml))
expect_equal(3, length(wb$vml_rels))
expect_equal(2, length(wb$vml_rels[[1]]))
expect_true(is.null(wb$vml_rels[[2]]))
expect_equal(1, length(wb$vml_rels[[3]]))
expect_length(wb$vml, 3)
expect_length(wb$vml_rels, 3)
expect_length(wb$vml_rels[[1]], 2)
expect_null(wb$vml_rels[[2]])
expect_length(wb$vml_rels[[3]], 1)

})

Expand Down
28 changes: 15 additions & 13 deletions tests/testthat/test-class-workbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ test_that("wb_set_col_widths", {
# set column width to 12
expect_silent(wb$set_col_widths("test", widths = 12L, cols = seq_along(mtcars)))
expect_equal(
"<col min=\"1\" max=\"11\" bestFit=\"1\" customWidth=\"1\" hidden=\"false\" width=\"12.711\"/>",
wb$worksheets[[1]]$cols_attr
wb$worksheets[[1]]$cols_attr,
"<col min=\"1\" max=\"11\" bestFit=\"1\" customWidth=\"1\" hidden=\"false\" width=\"12.711\"/>"
)

# wrong sheet
Expand All @@ -26,8 +26,8 @@ test_that("wb_set_col_widths", {
# reset the column with, we do not provide an option ot remove the column entry
expect_silent(wb$set_col_widths("test", cols = seq_along(mtcars)))
expect_equal(
"<col min=\"1\" max=\"11\" bestFit=\"1\" customWidth=\"1\" hidden=\"false\" width=\"9.141\"/>",
wb$worksheets[[1]]$cols_attr
wb$worksheets[[1]]$cols_attr,
"<col min=\"1\" max=\"11\" bestFit=\"1\" customWidth=\"1\" hidden=\"false\" width=\"9.141\"/>"
)

# create column width for column 25
Expand Down Expand Up @@ -360,7 +360,7 @@ test_that("clone worksheet", {
wb <- wb_load(fl)
# wb$get_sheet_names() # chartsheet has no named name?
expect_silent(wb$clone_worksheet(1, "Clone 1"))
expect_true(inherits(wb$worksheets[[5]], "wbChartSheet"))
expect_s3_class(wb$worksheets[[5]], "wbChartSheet")
# wb$open()

# clone pivot table and drawing -----------------------------------------
Expand Down Expand Up @@ -502,7 +502,7 @@ test_that("add_drawing works", {
add_drawing(xml = tmp, dims = NULL)$
add_drawing(xml = tmp, dims = "L19")

expect_equal(1L, length(wb$drawings))
expect_length(wb$drawings, 1L)

})

Expand Down Expand Up @@ -563,7 +563,7 @@ test_that("add_drawing works", {
wb <- wb %>%
wb_add_mschart(dims = "F4:L20", graph = scatter_plot)

expect_equal(1L, NROW(wb$charts))
expect_equal(NROW(wb$charts), 1L)

chart_01 <- ms_linechart(
data = us_indus_prod,
Expand Down Expand Up @@ -644,7 +644,7 @@ test_that("add_chartsheet works", {

wb$add_mschart(graph = data_plot)

expect_equal(1, nrow(wb$charts))
expect_equal(nrow(wb$charts), 1)

expect_true(grepl("A &amp; B", wb$charts$chart))

Expand All @@ -661,7 +661,7 @@ test_that("add_chartsheet works", {
)
wb$add_mschart(sheet = 2, graph = data_plot)

expect_equal(2L, nrow(wb$charts))
expect_equal(nrow(wb$charts), 2L)

exp <- "xdr:absoluteAnchor"
got <- xml_node_name(unlist(wb$drawings), "xdr:wsDr")
Expand Down Expand Up @@ -844,7 +844,7 @@ test_that("numfmt in pivot tables works", {
## Create the workbook and the pivot table
wb <- wb_workbook()$
add_worksheet("Data")$
add_data(x = df, startCol = 1, startRow = 2)
add_data(x = df, start_col = 1, start_row = 2)

df <- wb_data(wb)
wb$add_pivot_table(df, dims = "A3", rows = "cyl", cols = "gear",
Expand Down Expand Up @@ -883,7 +883,7 @@ test_that("numfmt in pivot tables works", {
## Create the workbook and the pivot table
wb <- wb_workbook()$
add_worksheet("Data")$
add_data(x = df, startCol = 1, startRow = 2)
add_data(x = df, start_col = 1, start_row = 2)

df <- wb_data(wb)
wb$add_pivot_table(
Expand Down Expand Up @@ -954,7 +954,9 @@ test_that("genBaseWorkbook() works", {
"smartTagTypes", "webPublishing", "fileRecoveryPr", "webPublishObjects",
"extLst"
)
got <- names(genBaseWorkbook())
expect_equal(exp, got)
expect_equal(
names(genBaseWorkbook()),
exp
)

})
24 changes: 12 additions & 12 deletions tests/testthat/test-cloneWorksheet.R
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ test_that("clone Worksheet with table", {

wb <- wb_workbook()
wb$add_worksheet("Sheet 1")
wb$add_data_table(sheet = "Sheet 1", x = iris, tableName = "iris")
wb$add_data_table(sheet = 1, x = mtcars, tableName = "mtcars", startCol = 10)
wb$add_data_table(sheet = "Sheet 1", x = iris, table_name = "iris")
wb$add_data_table(sheet = 1, x = mtcars, table_name = "mtcars", start_col = 10)

# FIXME a wild drawing2.xml appears in wb$Content_Types
wb$clone_worksheet("Sheet 1", "Clone 1")
Expand Down Expand Up @@ -67,7 +67,7 @@ test_that("copy cells", {
add_data(x = mtcars)$
add_fill(dims = "A1:F1", color = wb_color("yellow"))

dat <- wb_data(wb, dims = "A1:D4", colNames = FALSE)
dat <- wb_data(wb, dims = "A1:D4", col_names = FALSE)

# FIXME there is a bug with next_sheet() in clone_worksheets()
wb$
Expand All @@ -87,20 +87,20 @@ test_that("copy cells", {
clone_worksheet(old = 1, new = "Clone5")$
copy_cells(data = dat, dims = "A20", as_value = TRUE, as_ref = FALSE, transpose = TRUE)

got <- wb_data(wb, sheet = 2, dims = "M2:P5", colNames = FALSE)
got <- wb_data(wb, sheet = 2, dims = "M2:P5", col_names = FALSE)
expect_equal(dat, got, ignore_attr = TRUE)

got <- wb_data(wb, sheet = 3, dims = "A20:D23", colNames = FALSE)
got <- wb_data(wb, sheet = 3, dims = "A20:D23", col_names = FALSE)
expect_equal(unlist(t(dat)), unlist(got), ignore_attr = TRUE)

exp <- c("'Sheet 1'!A1", "'Sheet 1'!B1", "'Sheet 1'!C1", "'Sheet 1'!D1")
got <- wb_data(wb, sheet = 4, dims = "A20:D23", colNames = FALSE, showFormula = TRUE)[[1]]
got <- wb_data(wb, sheet = 4, dims = "A20:D23", col_names = FALSE, show_formula = TRUE)[[1]]
expect_equal(exp, got)

got <- wb_data(wb, sheet = 5, dims = "A20:D23", colNames = FALSE)
got <- wb_data(wb, sheet = 5, dims = "A20:D23", col_names = FALSE)
expect_equal(dat, got, ignore_attr = TRUE)

got <- wb_data(wb, sheet = 6, dims = "A20:D23", colNames = FALSE)
got <- wb_data(wb, sheet = 6, dims = "A20:D23", col_names = FALSE)
expect_equal(unlist(t(dat)), unlist(got), ignore_attr = TRUE)

})
Expand Down Expand Up @@ -170,7 +170,7 @@ test_that("wb_set_header_footer() works", {
test_that("cloning from workbooks works", {

## FIXME these tests should be improved, right now they only check the
## existance of a worksheet
## existence of a worksheet

# create a second workbook
wb <- wb_workbook()$
Expand All @@ -184,9 +184,8 @@ test_that("cloning from workbooks works", {
wb_in <- wb_load(fl)

wb$clone_worksheet(old = "SUM", new = "SUM", from = wb_in)
exp <- c("NOT_SUM", "SUM")
got <- wb$get_sheet_names() %>% unname()
expect_equal(exp, got)
expect_equal(got, c("NOT_SUM", "SUM"))

wb$clone_worksheet(old = "SUM", new = "SUM_clone")
exp <- c("NOT_SUM", "SUM", "SUM_clone")
Expand Down Expand Up @@ -284,7 +283,8 @@ test_that("cloning column and row styles works", {
wb$worksheets[[1]]$sheet_data$row_attr[2, "customFormat"] <- "1"
wb$worksheets[[1]]$sheet_data$row_attr[2, "s"] <- wb$styles_mgr$get_xf_id("new_styles")

cols <- openxlsx2:::wb_create_columns(wb, sheet = 1, cols = seq_along(mtcars))
# wb_create_columns is an internal function in openxlsx2.
cols <- wb_create_columns(wb, sheet = 1, cols = seq_along(mtcars))
cols[cols$min == 11, "style"] <- "1"
wb$worksheets[[1]]$fold_cols(cols)

Expand Down
8 changes: 4 additions & 4 deletions tests/testthat/test-conditional_formatting.R
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,8 @@ test_that("containsBlanks works", {

wb <- wb_workbook()
wb$add_worksheet()
wb$add_data(x = c(NA, 1, 2, ""), colNames = FALSE, na.strings = NULL)
wb$add_data(x = c(NA, 1, 2, ""), colNames = FALSE, na.strings = NULL, startCol = 2)
wb$add_data(x = c(NA, 1, 2, ""), col_names = FALSE, na.strings = NULL)
wb$add_data(x = c(NA, 1, 2, ""), col_names = FALSE, na.strings = NULL, start_col = 2)
wb$add_conditional_formatting(cols = 1, rows = 1:4, type = "containsBlanks")
wb$add_conditional_formatting(cols = 2, rows = 1:4, type = "notContainsBlanks")

Expand All @@ -715,7 +715,7 @@ test_that("warning on cols > 2 and dims", {
wb$add_conditional_formatting(
rows = seq_len(nrow(mtcars)),
cols = c(2, 4, 6),
type = 'between',
type = "between",
rule = c(2, 4)
),
"cols > 2, will create range from min to max."
Expand All @@ -724,7 +724,7 @@ test_that("warning on cols > 2 and dims", {
wb <- wb_workbook()$add_worksheet()$add_data(x = mtcars)
wb$add_conditional_formatting(
dims = "B2:F5",
type = 'between',
type = "between",
rule = c(2, 4)
)

Expand Down
30 changes: 11 additions & 19 deletions tests/testthat/test-converters.R
Original file line number Diff line number Diff line change
@@ -1,34 +1,28 @@
test_that("int2col", {

exp <- LETTERS[1:10]
got <- int2col(1:10)
expect_equal(exp, got)

expect_error(int2col("a"),
"x must be numeric.")

expect_equal(int2col(1:10), LETTERS[1:10])
expect_error(int2col("a"), "x must be numeric.")
})

test_that("col2int", {
expect_null(col2int(NULL))

expect_equal(1, col2int("a"))
expect_equal(1, col2int(1))
expect_equal(col2int("a"), 1)
expect_equal(col2int(1), 1)
expect_error(col2int(list()), "x must be character")

expect_equal(1, col2int("A"))
expect_equal(c(1, 3, 4), col2int(c("A", "C:D")))
expect_equal(c(1, 3, 4, 11), col2int(c("A", "C:D", "K")))
expect_equal(c(1, 3, 4, 11, 27, 28, 29, 30), col2int(c("A", "C:D", "K", "AA:AD")))
expect_equal(col2int("A"), 1)
expect_equal(col2int(c("A", "C:D")), c(1, 3, 4))
expect_equal(col2int(c("A", "C:D", "K")), c(1, 3, 4, 11))
expect_equal(col2int(c("A", "C:D", "K", "AA:AD")), c(1, 3, 4, 11, 27, 28, 29, 30))
expect_error(col2int(c("a", NA_character_, "c")), "x contains NA")

})

test_that("get_cell_refs", {

exp <- c("B1", "C2", "D3")
got <- get_cell_refs(data.frame(1:3, 2:4))
expect_equal(exp, got)
expect_equal(got, c("B1", "C2", "D3"))

expect_error(get_cell_refs(data.frame("a", "a")),
"cellCoords must only contain integers")
Expand Down Expand Up @@ -57,12 +51,10 @@ test_that("", {
)
on.exit(options(op), add = TRUE)

exp <- 10
got <- calc_col_width(wb_workbook()$get_base_font(), 11)
expect_equal(exp, got)
expect_equal(got, 10)

exp <- 8
got <- calc_col_width(wb_workbook()$get_base_font(), 7)
expect_equal(exp, got)
expect_equal(got, 8)

})
Loading
Loading