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.

Sign up for GitHub

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

Jump to bottom

Use public interface in tests wherever possible (don't use :::) #1692

Open
12 of 24 tasks
MichaelChirico opened this issue Oct 13, 2022 · 1 comment · May be fixed by #2801
Open
12 of 24 tasks

Use public interface in tests wherever possible (don't use :::) #1692

MichaelChirico opened this issue Oct 13, 2022 · 1 comment · May be fixed by #2801
Labels
internals Issues related to inner workings of lintr, i.e., not user-visible testing
Milestone
3.3.0

Comments

@MichaelChirico
Copy link
Collaborator

MichaelChirico commented Oct 13, 2022 •
edited
Loading

e.g. this suite can be replaced by expect_lint() tests:

lintr/tests/testthat/test-implicit_integer_linter.R

Lines 2 to 35 in bed54c5

test_that("single numerical constants are properly identified ", {
# Test single numerical constants
is_implicit <- lintr:::is_implicit_integer
x <- c("Inf", "NaN", "TRUE", "FALSE", "NA", "NA_character")
y <- c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)
expect_equal(is_implicit(x), y)
x <- c("2.000", "2.", "2L", "2.0", "2.1", "2")
y <- c(FALSE, FALSE, FALSE, FALSE, FALSE, TRUE)
expect_equal(is_implicit(x), y)
# 1000 1000L 1000L 1200* 0.0012 0.001 0.0... 1.2
x <- c("1e3", "1e3L", "1.0e3L", "1.2e3", "1.2e-3", "1e-3", "1e-33", "1.2e0")
y <- c(TRUE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE)
expect_equal(is_implicit(x), y)
# 1* 123L 123* 123.1
x <- c("0x1p+0", "0x1.ecp+6L", "0x1.ecp+6", "0x1.ec66666666666p+6")
y <- c(FALSE, FALSE, FALSE, FALSE)
expect_equal(is_implicit(x), y)
x <- c("8i", "8.0i")
y <- c(FALSE, FALSE)
expect_equal(is_implicit(x), y)
max <- .Machine[["integer.max"]] # largest number that R can represent as an integer
x <- as.character(c(-max - 1.0, -max, max, max + 1.0))
y <- c(FALSE, TRUE, TRUE, FALSE)
expect_equal(is_implicit(x), y)
# Note: cases indicated by "*" should be TRUE but they are complicated to handle, and it is not
# clear how users could keep these whole numbers represented as doubles without a lint.
})

Here's a list of tests using :::, generated with grep -Flr ":::" tests/testthat | awk -F/ '{print "- [ ] " $3}' | sort, we can cross some off if they are just used for coverage & testing with public API is difficult:
@MichaelChirico MichaelChirico added internals Issues related to inner workings of lintr, i.e., not user-visible testing labels Oct 13, 2022
@MichaelChirico MichaelChirico mentioned this issue Oct 13, 2022
Merged
@IndrajeetPatil
Copy link
Collaborator

IndrajeetPatil commented Oct 13, 2022 •
edited
Loading

I think test-namespace_linter.R, test-object-name-linter.R, and test-undesirable_operator_linter.R are false positives.

It's picking up ::: from tests where they are deliberate. E.g.

  expect_lint(
    "stats:::sdd(c(1,2,3))",
    list(message = rex::rex("'sdd' does not exist in {stats}")),
    linter
  )

@MichaelChirico MichaelChirico mentioned this issue Oct 14, 2022
Merged
@IndrajeetPatil IndrajeetPatil mentioned this issue Oct 16, 2022
Merged
IndrajeetPatil added a commit that referenced this issue Oct 16, 2022
@IndrajeetPatil
Use public interface to test internal functions: Part-2
7f53833 
Part of #1692
@IndrajeetPatil IndrajeetPatil mentioned this issue Oct 16, 2022
Merged
IndrajeetPatil added a commit that referenced this issue Oct 17, 2022
@IndrajeetPatil
Use public interface to test internal functions: Part-3
9a9affd 
Part of #1692

And cleanup to break the tests down into skip vs block blocks.
@IndrajeetPatil IndrajeetPatil mentioned this issue Oct 17, 2022
Merged
MichaelChirico added a commit that referenced this issue Oct 17, 2022
@IndrajeetPatil @MichaelChirico
Use public interface to test internal functions: Part-3 (#1726)
f5e8ea6 
* Use public interface to test internal functions: Part-3

Part of #1692

And cleanup to break the tests down into skip vs block blocks.

* remove escapes

Co-authored-by: Michael Chirico <[email protected]>
@IndrajeetPatil IndrajeetPatil mentioned this issue Nov 9, 2022
Merged
@AshesITR AshesITR mentioned this issue Nov 9, 2022
Closed
IndrajeetPatil added a commit that referenced this issue Nov 9, 2022
@IndrajeetPatil
Remove undesirable_operator_linter from .lintr_new
46797ed 
Closes #1766

Add it back once #1692 is resolved
@IndrajeetPatil IndrajeetPatil mentioned this issue Nov 9, 2022
Merged
IndrajeetPatil added a commit that referenced this issue Nov 9, 2022
@IndrajeetPatil
Remove undesirable_operator_linter() from .lintr_new (#1767)
1e9b93d 
* Remove `undesirable_operator_linter` from `.lintr_new`

Closes #1766

Add it back once #1692 is resolved

* Update test-settings.R

* remaining
IndrajeetPatil added a commit that referenced this issue Nov 21, 2022
@IndrajeetPatil
Use public interface to test internal functions: Part-4
2864d74 
Part of #1692
@IndrajeetPatil IndrajeetPatil mentioned this issue Nov 21, 2022
Merged
IndrajeetPatil added a commit that referenced this issue Nov 26, 2022
@IndrajeetPatil
Use public interface to test internal functions: Part-4 (#1774)
8e7bfc9 
* Use public interface to test internal functions: Part-4

Part of #1692

* fix lints

* check also the location

* fix URL

* fix for windows?

* Use `winslash = "/"`
@MichaelChirico MichaelChirico added this to the 3.1.2 milestone Oct 12, 2023
@MichaelChirico MichaelChirico modified the milestones: 3.2.0, 3.3.0 Dec 5, 2023
IndrajeetPatil added a commit that referenced this issue Jun 6, 2024
@IndrajeetPatil
Use public interface to test logical_env()
f2409ea 
cf. #1692
This was referenced Jun 6, 2024
Merged
Merged
@MichaelChirico MichaelChirico linked a pull request Feb 27, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
internals Issues related to inner workings of lintr, i.e., not user-visible testing
Projects
None yet
Milestone
3.3.0
Development

Successfully merging a pull request may close this issue.

Remove some tests of internal API r-lib/lintr
2 participants
@MichaelChirico @IndrajeetPatil