Skip to content

Commit

Permalink
Lints to tests (#430)
Browse files Browse the repository at this point in the history
* lints

* Use expect_true / false length

* expect_named

* depend on testthat 3.2.0
  • Loading branch information
olivroy authored Oct 21, 2024
1 parent 6f4612f commit d440d45
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 51 deletions.
4 changes: 2 additions & 2 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Authors@R: c(
Description: Work with XML files using a simple, consistent interface.
Built on top of the 'libxml2' C library.
License: MIT + file LICENSE
URL: https://xml2.r-lib.org/, https://github.com/r-lib/xml2
URL: https://xml2.r-lib.org, https://github.com/r-lib/xml2
BugReports: https://github.com/r-lib/xml2/issues
Depends:
R (>= 3.6.0)
Expand All @@ -28,7 +28,7 @@ Suggests:
magrittr,
mockery,
rmarkdown,
testthat (>= 3.0.0)
testthat (>= 3.2.0)
VignetteBuilder:
knitr
Config/Needs/website: tidyverse/tidytemplate
Expand Down
5 changes: 2 additions & 3 deletions R/format.R
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
#' @export
format.xml_node <- function(x, ...) {
attrs <- xml_attrs(x)
paste("<",
paste0("<",
paste(
c(
xml_name(x),
format_attributes(attrs)
),
collapse = " "
),
">",
sep = ""
">"
)
}

Expand Down
2 changes: 1 addition & 1 deletion R/paths.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ path_to_connection <- function(path, check = c("file", "dir")) {
}

if (is_url(path)) {
if (requireNamespace("curl", quietly = TRUE)) {
if (is_installed("curl")) {
return(curl::curl(path))
} else {
return(url(path))
Expand Down
4 changes: 0 additions & 4 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ quote_str <- function(x, quote = "\"") {
paste0(quote, x, quote)
}

is_installed <- function(pkg) {
requireNamespace(pkg, quietly = TRUE)
}

# Format the C bitwise flags for display in Rd. The input object is a named
# integer vector with a 'descriptions' character vector attribute that
# corresponds to each flag.
Expand Down
2 changes: 1 addition & 1 deletion man/xml2-package.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 18 additions & 18 deletions tests/testthat/test-xml_attr.R
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ test_that("qualified names returned when ns given", {
bars <- xml_children(xml_children(x))
attr <- xml_attrs(bars, ns)

expect_equal(names(attr[[1]]), "f:id")
expect_equal(names(attr[[2]]), "g:id")
expect_named(attr[[1]], "f:id")
expect_named(attr[[2]], "g:id")
})


Expand Down Expand Up @@ -150,21 +150,21 @@ test_that("xml_attr<- removes namespaces if desired", {
x <- read_xml("<a xmlns = 'tag:foo'><b/></a>")

# cannot find //b with a default namespace
expect_equal(length(xml_find_all(x, "//b")), 0)
expect_length(xml_find_all(x, "//b"), 0)

# unless we specify it explicitly
expect_equal(length(xml_find_all(x, "//b")), 0)
expect_equal(length(xml_find_all(x, "//d1:b", xml_ns(x))), 1)
expect_length(xml_find_all(x, "//b"), 0)
expect_length(xml_find_all(x, "//d1:b", xml_ns(x)), 1)

# but can find it once we remove the namespace
xml_attr(x, "xmlns") <- NULL
expect_equal(length(xml_find_all(x, "//b")), 1)
expect_length(xml_find_all(x, "//b"), 1)

# and add the old namespace back
xml_attr(x, "xmlns") <- "tag:foo"
expect_equal(xml_attr(x, "xmlns"), "tag:foo")
expect_equal(length(xml_find_all(x, "//b")), 0)
expect_equal(length(xml_find_all(x, "//d1:b", xml_ns(x))), 1)
expect_length(xml_find_all(x, "//b"), 0)
expect_length(xml_find_all(x, "//d1:b", xml_ns(x)), 1)

expect_equal(xml_attr(x, "xmlns"), "tag:foo")
})
Expand All @@ -173,22 +173,22 @@ test_that("xml_attr<- removes prefixed namespaces if desired", {
x <- read_xml("<a xmlns:pre = 'tag:foo'><pre:b/></a>")

# cannot find //b with a prefixed namespace
expect_equal(length(xml_find_all(x, "//b")), 0)
expect_length(xml_find_all(x, "//b"), 0)

# unless we specify it explicitly
expect_equal(length(xml_find_all(x, "//b")), 0)
expect_equal(length(xml_find_all(x, "//pre:b", xml_ns(x))), 1)
expect_length(xml_find_all(x, "//b"), 0)
expect_length(xml_find_all(x, "//pre:b", xml_ns(x)), 1)

# but can find it once we remove the namespace
xml_attr(x, "xmlns:pre") <- NULL
expect_equal(length(xml_find_all(x, "//b")), 1)
expect_length(xml_find_all(x, "//b"), 1)

# and add the old namespace back
xml_attr(x, "xmlns:pre") <- "tag:foo"
xml_set_namespace(xml_children(x)[[1]], "pre")
expect_equal(xml_attr(x, "xmlns:pre"), "tag:foo")
expect_equal(length(xml_find_all(x, "//b")), 0)
expect_equal(length(xml_find_all(x, "//pre:b", xml_ns(x))), 1)
expect_length(xml_find_all(x, "//b"), 0)
expect_length(xml_find_all(x, "//pre:b", xml_ns(x)), 1)

expect_equal(xml_attr(x, "xmlns:pre"), "tag:foo")
})
Expand All @@ -213,8 +213,8 @@ test_that("xml_set_attr works identically to xml_attr<-", {

# No errors for xml_missing
mss <- xml_find_first(bx, "./c")
expect_error(xml_attr(mss[[2]], "b") <- "blah", NA)
expect_error(xml_set_attr(mss[[2]], "b", "blah"), NA)
expect_no_error(xml_attr(mss[[2]], "b") <- "blah")
expect_no_error(xml_set_attr(mss[[2]], "b", "blah"))
})

test_that("xml_set_attrs works identically to xml_attrs<-", {
Expand All @@ -237,8 +237,8 @@ test_that("xml_set_attrs works identically to xml_attrs<-", {

# No errors for xml_missing
mss <- xml_find_first(bx, "./c")
expect_error(xml_attrs(mss[[2]]) <- c("b" = "blah"), NA)
expect_error(xml_set_attrs(mss[[2]], c("b" = "blah")), NA)
expect_no_error(xml_attrs(mss[[2]]) <- c("b" = "blah"))
expect_no_error(xml_set_attrs(mss[[2]], c("b" = "blah")))
})

test_that("xml_set_attr can set the same namespace multiple times", {
Expand Down
20 changes: 10 additions & 10 deletions tests/testthat/test-xml_find.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,23 @@ test_that("xml_find_first does not deduplicate identical results", {
y <- xml_find_all(x, ".//y")
z <- xml_find_first(y, "..")
expect_s3_class(z, "xml_nodeset")
expect_equal(length(z), 2)
expect_length(z, 2)
})

# Find all ---------------------------------------------------------------------

test_that("unqualified names don't look in default ns", {
x <- read_xml(test_path("ns-multiple-default.xml"))

expect_equal(length(xml_find_all(x, "//bar")), 0)
expect_length(xml_find_all(x, "//bar"), 0)
})

test_that("qualified names matches to namespace", {
x <- read_xml(test_path("ns-multiple-default.xml"))
ns <- xml_ns(x)

expect_equal(length(xml_find_all(x, "//d1:bar", ns)), 1)
expect_equal(length(xml_find_all(x, "//d2:bar", ns)), 1)
expect_length(xml_find_all(x, "//d1:bar", ns), 1)
expect_length(xml_find_all(x, "//d2:bar", ns), 1)
})

test_that("warning if unknown namespace", {
Expand All @@ -45,7 +45,7 @@ test_that("warning if unknown namespace", {

test_that("no matches returns empty nodeset", {
x <- read_xml("<foo><bar /></foo>")
expect_equal(length(xml_find_all(x, "//baz")), 0)
expect_length(xml_find_all(x, "//baz"), 0)
})

test_that("xml_find_all returns nodeset or list of nodesets based on flatten", {
Expand Down Expand Up @@ -145,15 +145,15 @@ test_that("xml_find_lgl errors with non logical results", {
test_that("xml_find_lgl returns a logical result", {
x <- read_xml("<x><y>1</y><y/></x>")

expect_equal(xml_find_lgl(x, "1=1"), TRUE)
expect_true(xml_find_lgl(x, "1=1"))

expect_equal(xml_find_lgl(x, "1!=1"), FALSE)
expect_false(xml_find_lgl(x, "1!=1"))

expect_equal(xml_find_lgl(x, "true()=true()"), TRUE)
expect_true(xml_find_lgl(x, "true()=true()"))

expect_equal(xml_find_lgl(x, "true()=false()"), FALSE)
expect_false(xml_find_lgl(x, "true()=false()"))

expect_equal(xml_find_lgl(x, "'test'='test'"), TRUE)
expect_true(xml_find_lgl(x, "'test'='test'"))
})

test_that("Searches with empty inputs retain type stability", {
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-xml_missing.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ b <- xml_find_first(para, ".//b")
mss <- b[[3]]

test_that("xml_find returns nodes of class 'xml_missing' for missing nodes", {
expect_equal(length(b), 3L)
expect_equal(vapply(b, length, integer(1)), c(2L, 2L, 0L))
expect_length(b, 3L)
expect_equal(lengths(b), c(2L, 2L, 0L))
expect_s3_class(mss, "xml_missing")
})

Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/test-xml_namespaces.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,18 @@ test_that("unique prefix-url combo unchanged", {

test_that("all prefixs kept", {
x <- c(blah = "http://blah.com", rah = "http://blah.com")
expect_equal(names(.Call(unique_ns, x)), c("blah", "rah"))
expect_named(.Call(unique_ns, x), c("blah", "rah"))
})

test_that("multiple default namespaces can be stripped", {
x <- read_xml(test_path("ns-multiple-default.xml"))
ns <- unclass(xml_ns(x))
expect_equal(ns, c(d1 = "http://foo.com", d2 = "http://bar.com"))
expect_equal(length(xml_find_all(x, "//bar")), 0)
expect_length(xml_find_all(x, "//bar"), 0)

xml_ns_strip(x)
ns <- unclass(xml_ns(x))

expect_equal(unname(ns), character())
expect_equal(length(xml_find_all(x, "//bar")), 2)
expect_length(xml_find_all(x, "//bar"), 2)
})
4 changes: 2 additions & 2 deletions tests/testthat/test-xml_nodeset.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ test_that("methods work on empty nodesets", {
expect_identical(as_list(empty), list())
expect_identical(nodeset_apply(empty, identical), empty)
expect_output(print(empty), "\\{xml_nodeset \\(0\\)\\}")
expect_output(tree_structure(empty), NA)
expect_silent(tree_structure(empty))

xml_add_child(test, "test")
expect_identical(test, empty)
Expand Down Expand Up @@ -64,7 +64,7 @@ test_that("methods work on empty nodesets", {
expect_identical(test, empty)

expect_identical(xml_siblings(empty), empty)
expect_output(xml_structure(empty), NA)
expect_silent(xml_structure(empty))

expect_identical(xml_text(empty), character())
expect_identical(xml_url(empty), character())
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-xml_parse.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ test_that("read_xml errors with an empty document", {

test_that("read_html correctly parses malformed document", {
lego <- read_html(test_path("lego.html.bz2"))
expect_equal(length(xml_find_all(lego, ".//p")), 39)
expect_length(xml_find_all(lego, ".//p"), 39)
})

test_that("parse_options errors when given an invalid option", {
Expand Down Expand Up @@ -68,7 +68,7 @@ test_that("read_xml works with httr response objects", {

expect_s3_class(x, "xml_document")

expect_equal(length(xml_find_all(x, "//slide")), 2)
expect_length(xml_find_all(x, "//slide"), 2)
})

test_that("read_xml and read_html fail for bad status codes", {
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-xml_serialize.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ x <- read_xml("<a>
<b><c>123</c></b>
<b><c>456</c></b>
</a>")

test_that("xml_serialize and xml_unserialize work with xml_document input", {
out <- xml_unserialize(xml_serialize(x, NULL))
expect_identical(as.character(x), as.character(out))
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-xml_write.R
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,6 @@ test_that("write_xml returns invisibly", {

res <- withVisible(write_xml(x, tf))

expect_equal(res$value, NULL)
expect_equal(res$visible, FALSE)
expect_null(res$value)
expect_false(res$visible)
})

0 comments on commit d440d45

Please sign in to comment.