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

Remove some tests of internal API #2801

Draft
wants to merge 28 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
215e2e0
remove internal API test
MichaelChirico Feb 27, 2025
15cde56
remove more tests of internal helpers
MichaelChirico Feb 27, 2025
786e804
nocov
MichaelChirico Feb 27, 2025
0d0d051
unneeded read_settings() calls
MichaelChirico Feb 27, 2025
fafa8da
trim_output is not used
MichaelChirico Feb 27, 2025
ab86a5b
rm unused is_relative_path
MichaelChirico Feb 27, 2025
d165068
remove now-dead test
MichaelChirico Feb 27, 2025
9541270
Merge branch 'main' into rm-make-linter-test
MichaelChirico Feb 27, 2025
036bdd7
use public API in linter_tags
MichaelChirico Feb 27, 2025
aaf4620
Merge remote-tracking branch 'origin/rm-make-linter-test' into rm-mak…
MichaelChirico Feb 27, 2025
5789d8a
progress removing from object_name_linter tests
MichaelChirico Feb 27, 2025
11ea6fe
missing ','
MichaelChirico Feb 27, 2025
2d41ad5
Merge remote-tracking branch 'origin/rm-make-linter-test' into rm-mak…
MichaelChirico Feb 27, 2025
73c06aa
placehold ignore_order=TRUE for now
MichaelChirico Feb 27, 2025
8a7f524
Add ignore_order= argument
MichaelChirico Feb 27, 2025
ce923f9
remove file of all ::: tests
MichaelChirico Feb 28, 2025
ce7b2f2
better "wrong columns" test input
MichaelChirico Feb 28, 2025
e80a561
delint
MichaelChirico Mar 3, 2025
5717cb5
delete ::: tests to see what needs coverage
MichaelChirico Mar 3, 2025
58e830b
Merge remote-tracking branch 'origin/rm-make-linter-test' into rm-mak…
MichaelChirico Mar 3, 2025
c49ec12
Remove ':::' usage
MichaelChirico Mar 3, 2025
3cef71f
Remove mostly-internal test file
MichaelChirico Mar 3, 2025
360901a
Just delete more tests for now
MichaelChirico Mar 3, 2025
28bacc7
Merge branch 'main' into rm-make-linter-test
MichaelChirico Mar 3, 2025
6a30a9e
delint
MichaelChirico Mar 3, 2025
83ee84b
Merge remote-tracking branch 'origin/rm-make-linter-test' into rm-mak…
MichaelChirico Mar 3, 2025
23818cb
Just delete ::: tests
MichaelChirico Mar 3, 2025
24e6e54
test was picking up global settings (?)
MichaelChirico Mar 3, 2025
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
4 changes: 2 additions & 2 deletions R/make_linter_from_regex.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
make_linter_from_regex <- function(regex,
lint_type,
lint_msg) {
function() {
function() { # nocov: only run at namespace load time
Linter(linter_level = "file", function(source_expression) {
all_matches <- re_matches(
source_expression[["file_lines"]],
Expand Down Expand Up @@ -29,7 +29,7 @@ make_linter_from_regex <- function(regex,
})
lints[lengths(lints) > 0L]
})
}
} # nocov: ditto opening brace
}

#' Determine if a regex match is covered by an expression in a source_expression
Expand Down
33 changes: 0 additions & 33 deletions R/methods.R
Original file line number Diff line number Diff line change
Expand Up @@ -85,39 +85,6 @@ print.lints <- function(x, ...) {
invisible(x)
}

trim_output <- function(x, max = 65535L) {
# if x is less than the max, just return it
if (length(x) <= 0L || nchar(x) <= max) {
return(x)
}

# otherwise trim x to the max, then search for the lint starts
x <- substr(x, 1L, max)

re <- rex(
"[", except_some_of(":"), ":", numbers, ":", numbers, ":", "]",
"(", except_some_of(")"), ")",
space,
"*", or("style", "warning", "error"), ":", "*",
except_some_of("\r\n"), newline,
except_some_of("\r\n"), newline,
except_some_of("\r\n"), newline,
except_some_of("\r\n"), newline,
except_some_of("\r\n"), newline
)

lint_starts <- re_matches(x, re, global = TRUE, locations = TRUE)[[1L]]

# if at least one lint ends before the cutoff, cutoff there, else just use
# the cutoff
last_end <- lint_starts[NROW(lint_starts), "end"]
if (!is.na(last_end)) {
substr(x, 1L, last_end)
} else {
x
}
}

#' @export
names.lints <- function(x, ...) {
vapply(x, `[[`, character(1L), "filename")
Expand Down
4 changes: 0 additions & 4 deletions R/path_utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ is_root_path <- function(path) {
re_matches(path, root_path_regex)
}

is_relative_path <- function(path) {
re_matches(path, relative_path_regex)
}

is_path <- function(path) {
re_matches(path, path_regex)
}
Expand Down
22 changes: 0 additions & 22 deletions tests/testthat/test-absolute_path_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -52,28 +52,6 @@ test_that("is_absolute_path", {
expect_identical(f(x), y)
})


test_that("is_relative_path", {
f <- lintr:::is_relative_path

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

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

x <- c("/foo", "foo/", "foo/bar", "foo//bar", "./foo", "../foo")
y <- c(FALSE, TRUE, TRUE, TRUE, TRUE, TRUE)
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_identical(f(x), y)
})


test_that("is_path", {
f <- lintr:::is_path

Expand Down
2 changes: 0 additions & 2 deletions tests/testthat/test-lint_dir.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ test_that("lint all relevant directories in a package", {
c("package.Rproj", "DESCRIPTION", "NAMESPACE", "lintr_test_config")
)

lintr:::read_settings(NULL)
lints <- lint_package(the_pkg, parse_settings = FALSE)
linted_files <- unique(names(lints))

Expand All @@ -34,7 +33,6 @@ test_that("lint all relevant directories in a package", {
# We want to ensure that object_name_linter uses namespace_imports correctly.
# assignment_linter is needed to cause a lint in all vignettes.
linters <- list(assignment_linter(), object_name_linter())
lintr:::read_settings(NULL)
lints <- lint_package(the_pkg, linters = linters, parse_settings = FALSE)
linted_files <- unique(names(lints))

Expand Down
5 changes: 0 additions & 5 deletions tests/testthat/test-make_linter_from_regex.R

This file was deleted.

12 changes: 0 additions & 12 deletions tests/testthat/test-methods.R
Original file line number Diff line number Diff line change
@@ -1,21 +1,9 @@
test_that("it returns the input if less than the max", {
expect_identical(lintr:::trim_output(character()), character())
expect_identical(lintr:::trim_output("test", max = 10L), "test")
})

test_that("it returns the input trimmed strictly to max if no lints found", {
expect_identical(lintr:::trim_output("testing a longer non_lint string", max = 7L), "testing")
})

test_that("it returns the input trimmed to the last full lint if one exists within the max", {
t1 <- readChar(test_path("lints"), file.size(test_path("lints")))
if (.Platform$OS.type == "windows") {
# Magic numbers expect newlines to be 1 character
t1 <- gsub("\r\n", "\n", t1, fixed = TRUE)
}
expect_identical(lintr:::trim_output(t1, max = 200L), substr(t1, 1L, 195L))
expect_identical(lintr:::trim_output(t1, max = 400L), substr(t1, 1L, 380L))
expect_identical(lintr:::trim_output(t1, max = 2000L), substr(t1, 1L, 1930L))
})

test_that("as.data.frame.lints", {
Expand Down
Loading