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

Wherever possible, prefer expect_identical() over expect_equal() #1712

Merged
merged 3 commits into from
Oct 15, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 25 additions & 25 deletions tests/testthat/test-absolute_path_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,27 @@ test_that("is_root_path", {

x <- character()
y <- logical()
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("", "foo", "http://rseek.org/", "./", " /", "/foo", "'/'")
y <- c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("/", "//")
y <- c(TRUE, FALSE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("~", "~/", "~//", "~bob2", "~foo_bar/")
y <- c(TRUE, TRUE, TRUE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("c:", "C:\\", "D:/", "C:\\\\", "D://")
y <- c(TRUE, TRUE, TRUE, FALSE, FALSE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("\\\\", "\\\\localhost", "\\\\localhost\\")
y <- c(TRUE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)
})


Expand All @@ -33,23 +33,23 @@ test_that("is_absolute_path", {

x <- character()
y <- logical()
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("/", "//", "/foo", "/foo/")
y <- c(TRUE, FALSE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("~", "~/foo", "~/foo/", "~'")
y <- c(TRUE, TRUE, TRUE, FALSE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("c:", "C:\\foo\\", "C:/foo/")
y <- c(TRUE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("\\\\", "\\\\localhost", "\\\\localhost\\c$", "\\\\localhost\\c$\\foo")
y <- c(TRUE, TRUE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)
})


Expand All @@ -58,19 +58,19 @@ test_that("is_relative_path", {

x <- character()
y <- logical()
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("/", "c:\\", "~/", "foo", "http://rseek.org/", "'./'")
y <- c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("/foo", "foo/", "foo/bar", "foo//bar", "./foo", "../foo")
y <- c(FALSE, TRUE, TRUE, TRUE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("\\\\", "\\foo", "foo\\", "foo\\bar", ".\\foo", "..\\foo", ".", "..", "../")
y <- c(FALSE, FALSE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)
})


Expand All @@ -79,15 +79,15 @@ test_that("is_path", {

x <- character()
y <- logical()
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("", "foo", "http://rseek.org/", "foo\nbar", "'foo/bar'", "'/'")
y <- c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("c:", "..", "foo/bar", "foo\\bar", "~", "\\\\localhost")
y <- c(TRUE, TRUE, TRUE, TRUE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)
})


Expand All @@ -96,23 +96,23 @@ test_that("is_valid_path", {

x <- character()
y <- logical()
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("C:/asdf", "C:/asd*f", "a\\s:df", "a\\\nsdf")
y <- c(TRUE, FALSE, FALSE, FALSE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("C:/asdf", "C:/asd*f", "a\\s:df", "a\\\nsdf")
y <- c(TRUE, FALSE, FALSE, FALSE)
expect_equal(f(x, lax = TRUE), y)
expect_identical(f(x, lax = TRUE), y)

x <- c("/asdf", "/asd*f", "/as:df", "/a\nsdf")
y <- c(TRUE, TRUE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("/asdf", "/asd*f", "/as:df", "/a\nsdf")
y <- c(TRUE, FALSE, FALSE, FALSE)
expect_equal(f(x, lax = TRUE), y)
expect_identical(f(x, lax = TRUE), y)
})


Expand All @@ -121,11 +121,11 @@ test_that("is_long_path", {

x <- character()
y <- logical()
expect_equal(f(x), y)
expect_identical(f(x), y)

x <- c("foo/", "/foo", "n/a", "Z:\\foo", "foo/bar", "~/foo", "../foo")
y <- c(FALSE, FALSE, FALSE, TRUE, TRUE, TRUE, TRUE)
expect_equal(f(x), y)
expect_identical(f(x), y)
})
# styler: on

Expand Down
52 changes: 26 additions & 26 deletions tests/testthat/test-cache.R
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ test_that("clear_cache deletes the file if a file is given", {
lintr:::save_cache(cache = e1, file = f1, path = d1)

want <- list(file.path(d1, fhash("R/test.R")), recursive = TRUE)
expect_equal(clear_cache(f1, d1), want)
expect_equal(clear_cache(file = f1, path = d1), want)
expect_identical(clear_cache(f1, d1), want)
expect_identical(clear_cache(file = f1, path = d1), want)
})

test_that("clear_cache deletes the directory if no file is given", {
mockery::stub(clear_cache, "read_settings", function(...) invisible(...))
mockery::stub(clear_cache, "unlink", function(...) list(...))

expect_equal(clear_cache(file = NULL, path = "."), list(".", recursive = TRUE))
expect_identical(clear_cache(file = NULL, path = "."), list(".", recursive = TRUE))
})

# `load_cache`
Expand All @@ -60,7 +60,7 @@ test_that("load_cache loads the saved file in a new empty environment", {
lintr:::save_cache(cache = e1, file = f1, path = d1)
e2 <- lintr:::load_cache(file = f1, path = d1)

expect_equal(e2, e1)
expect_identical(e2, e1)
})

test_that("load_cache returns an empty environment if no cache file exists", {
Expand All @@ -72,7 +72,7 @@ test_that("load_cache returns an empty environment if no cache file exists", {
lintr:::save_cache(cache = e1, file = f1, path = d1)
e2 <- lintr:::load_cache(file = f2, path = d1)

expect_equal(e2, e1)
expect_identical(e2, e1)
})

test_that("load_cache returns an empty environment if reading cache file fails", {
Expand All @@ -88,8 +88,8 @@ test_that("load_cache returns an empty environment if reading cache file fails",
saveRDS(e1, cache_f1)
expect_warning(e3 <- lintr:::load_cache(file = f1, path = d1), "Could not load cache file")

expect_equal(ls(e2), character())
expect_equal(ls(e3), character())
expect_identical(ls(e2), character())
expect_identical(ls(e3), character())
})

# `save_cache`
Expand Down Expand Up @@ -139,7 +139,7 @@ test_that("save_cache saves all non-hidden objects from the environment", {
e2 <- new.env(parent = emptyenv())
load(file = file.path(d1, fhash(f1)), envir = e2)

expect_equal(e2, e1)
expect_identical(e2, e1)
})

# `cache_file`
Expand Down Expand Up @@ -182,7 +182,7 @@ test_that("retrieve_file returns the cached result if found", {
f1 <- withr::local_tempfile(lines = "foobar")

lintr:::cache_file(e1, f1, list(), list("foobar"))
expect_equal(lintr:::retrieve_file(e1, f1, list()), list("foobar"))
expect_identical(lintr:::retrieve_file(e1, f1, list()), list("foobar"))
})

# `cache_lint`
Expand Down Expand Up @@ -229,7 +229,7 @@ test_that("retrieve_lint returns the same lints if nothing has changed", {
lines = test_data[["lines"]]
)

expect_equal(t1, test_data[["lints"]])
expect_identical(t1, test_data[["lints"]])
})

test_that(
Expand Down Expand Up @@ -257,9 +257,9 @@ test_that(
lines = lines2
)

expect_equal(t1[[1L]]$line_number, lints[[1L]]$line_number + 1L)
expect_equal(t1[[2L]]$line_number, lints[[2L]]$line_number + 1L)
expect_equal(t1[[3L]]$line_number, lints[[3L]]$line_number + 1L)
expect_identical(t1[[1L]]$line_number, lints[[1L]]$line_number + 1L)
expect_identical(t1[[2L]]$line_number, lints[[2L]]$line_number + 1L)
expect_identical(t1[[3L]]$line_number, lints[[3L]]$line_number + 1L)
}
)

Expand All @@ -285,7 +285,7 @@ test_that("retrieve_lint returns the same lints with lines added below", {
lines = lines2
)

expect_equal(t1, test_data[["lints"]])
expect_identical(t1, test_data[["lints"]])
})

test_that(
Expand Down Expand Up @@ -314,9 +314,9 @@ test_that(
lines = lines2
)

expect_equal(t1[[1L]]$line_number, lints1[[1L]]$line_number)
expect_equal(t1[[2L]]$line_number, lints1[[2L]]$line_number + 1L)
expect_equal(t1[[3L]]$line_number, lints1[[3L]]$line_number + 1L)
expect_identical(t1[[1L]]$line_number, lints1[[1L]]$line_number)
expect_identical(t1[[2L]]$line_number, lints1[[2L]]$line_number + 1L)
expect_identical(t1[[3L]]$line_number, lints1[[3L]]$line_number + 1L)
}
)

Expand Down Expand Up @@ -357,11 +357,11 @@ test_that("find_new_line returns the same if the line is the same", {
"foobar2",
"foobar3"
)
expect_equal(lintr:::find_new_line(1L, "foobar1", t1), 1L)
expect_identical(lintr:::find_new_line(1L, "foobar1", t1), 1L)

expect_equal(lintr:::find_new_line(2L, "foobar2", t1), 2L)
expect_identical(lintr:::find_new_line(2L, "foobar2", t1), 2L)

expect_equal(lintr:::find_new_line(3L, "foobar3", t1), 3L)
expect_identical(lintr:::find_new_line(3L, "foobar3", t1), 3L)
})

test_that("find_new_line returns the correct line if it is before the current line", {
Expand All @@ -370,11 +370,11 @@ test_that("find_new_line returns the correct line if it is before the current li
"foobar2",
"foobar3"
)
expect_equal(lintr:::find_new_line(1L, "foobar1", t1), 1L)
expect_identical(lintr:::find_new_line(1L, "foobar1", t1), 1L)

expect_equal(lintr:::find_new_line(2L, "foobar1", t1), 1L)
expect_identical(lintr:::find_new_line(2L, "foobar1", t1), 1L)

expect_equal(lintr:::find_new_line(3L, "foobar1", t1), 1L)
expect_identical(lintr:::find_new_line(3L, "foobar1", t1), 1L)
})

test_that("find_new_line returns the correct line if it is after the current line", {
Expand All @@ -383,11 +383,11 @@ test_that("find_new_line returns the correct line if it is after the current lin
"foobar2",
"foobar3"
)
expect_equal(lintr:::find_new_line(1L, "foobar3", t1), 3L)
expect_identical(lintr:::find_new_line(1L, "foobar3", t1), 3L)

expect_equal(lintr:::find_new_line(2L, "foobar3", t1), 3L)
expect_identical(lintr:::find_new_line(2L, "foobar3", t1), 3L)

expect_equal(lintr:::find_new_line(3L, "foobar3", t1), 3L)
expect_identical(lintr:::find_new_line(3L, "foobar3", t1), 3L)
})

#
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-comments.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ test_that("it determines Jenkins PR build info", {
)
expect_true(lintr:::in_ci())

expect_equal(lintr:::ci_build_info(), list(
expect_identical(lintr:::ci_build_info(), list(
user = "user",
repo = "repo",
pull = "123",
Expand All @@ -64,7 +64,7 @@ test_that("it determines Jenkins commit build info", {
)

expect_true(lintr:::in_ci())
expect_equal(lintr:::ci_build_info(), list(
expect_identical(lintr:::ci_build_info(), list(
user = "user",
repo = "repo",
pull = NULL,
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-exclusions.R
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test_that("it excludes properly", {
for (info in sprintf("caching: pass %s", 1L:4L)) {
t4 <- lint("exclusions-test", cache = cache_path, parse_settings = FALSE)

expect_equal(length(t4), 8L, info = info)
expect_identical(length(t4), 8L, info = info)
}
})

Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/test-ids_with_token.R
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
test_that("ids_with_token works as expected", {
source_expression <- get_source_expressions("tmp.R", "a <- 42L")$expressions[[1L]]
ref <- ids_with_token(source_expression = source_expression, value = "expr")
expect_equal(ref, c(1L, 3L, 6L))
expect_equal(source_expression$parsed_content$token[ref], rep_len("expr", length(ref)))
expect_identical(ref, c(1L, 3L, 6L))
expect_identical(source_expression$parsed_content$token[ref], rep_len("expr", length(ref)))

# deprecated argument
expect_warning(
old_arg <- ids_with_token(source_file = source_expression, value = "expr"),
"Argument source_file was deprecated"
)
expect_equal(old_arg, ref)
expect_identical(old_arg, ref)
})
16 changes: 8 additions & 8 deletions tests/testthat/test-lint_file.R
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ test_that("lint() results do not depend on the working directory", {
lint_assignments("jkl.R")
)

expect_equal(
expect_identical(
as.data.frame(lints_from_pkg_root)[["line"]],
expected_lines
)
expect_equal(
expect_identical(
as.data.frame(lints_from_outside),
as.data.frame(lints_from_pkg_root)
)
expect_equal(
expect_identical(
as.data.frame(lints_from_a_subdir),
as.data.frame(lints_from_pkg_root)
)
Expand Down Expand Up @@ -91,10 +91,10 @@ test_that("lint() results do not depend on the position of the .lintr", {
)
)

expect_equal(
expect_identical(
as.data.frame(lints_with_config_at_pkg_root)[["line"]], expected_lines
)
expect_equal(
expect_identical(
as.data.frame(lints_with_config_at_pkg_root),
as.data.frame(lints_with_config_in_r_dir),
info = paste(
Expand Down Expand Up @@ -129,16 +129,16 @@ test_that("lint() results from file or text should be consistent", {
expect_length(lint_from_text, 2L)
expect_length(lint_from_text2, 2L)

expect_equal(lint_from_file, lint_from_text2)
expect_identical(lint_from_file, lint_from_text2)

for (i in seq_along(lint_from_lines)) {
lint_from_file[[i]]$filename <- ""
lint_from_lines[[i]]$filename <- ""
lint_from_text[[i]]$filename <- ""
}

expect_equal(lint_from_file, lint_from_lines)
expect_equal(lint_from_file, lint_from_text)
expect_identical(lint_from_file, lint_from_lines)
expect_identical(lint_from_file, lint_from_text)
})

test_that("exclusions work with custom linter names", {
Expand Down
Loading