From aa2fee497a449b0f9e849aff887c28aabcd9f062 Mon Sep 17 00:00:00 2001 From: wlandau Date: Fri, 19 Aug 2022 11:07:29 -0400 Subject: [PATCH] Lints and checks --- .github/workflows/check.yaml | 78 +++++++------------ .github/workflows/cover.yaml | 60 +++++--------- .github/workflows/lint.yaml | 40 +++------- .lintr | 1 + R/walk_code.R | 6 +- .../templates/drake/skeleton/skeleton.Rmd | 2 +- inst/testing/knitr/test.Rmd | 4 +- tests/testthat/test-4-flow.R | 10 ++- tests/testthat/test-6-analysis.R | 6 +- tests/testthat/test-6-plans.R | 6 +- tests/testthat/test-7-dsl.R | 8 +- tests/testthat/test-9-future.R | 2 +- 12 files changed, 84 insertions(+), 139 deletions(-) diff --git a/.github/workflows/check.yaml b/.github/workflows/check.yaml index 672f9e200..df097d6ab 100644 --- a/.github/workflows/check.yaml +++ b/.github/workflows/check.yaml @@ -1,3 +1,5 @@ +# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples +# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help on: [push, pull_request] name: check @@ -12,69 +14,45 @@ jobs: fail-fast: false matrix: config: - - {os: windows-latest, r: '4.0'} - - {os: macOS-latest, r: 'release'} - - {os: ubuntu-20.04, r: 'release', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest"} + - {os: macOS-latest, r: 'release'} + - {os: windows-latest, r: 'release'} + - {os: ubuntu-latest, r: 'devel', http-user-agent: 'release'} + - {os: ubuntu-latest, r: 'release'} + - {os: ubuntu-latest, r: 'oldrel-1'} env: - R_REMOTES_NO_ERRORS_FROM_WARNINGS: true - RSPM: ${{ matrix.config.rspm }} - GITHUB_PAT: ${{ secrets.GITHUBPAT }} + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + NOT_CRAN: true + R_KEEP_PKG_SOURCE: yes + R_REMOTES_NO_ERRORS_FROM_WARNINGS: false + TORCH_INSTALL: 1 steps: - uses: actions/checkout@v2 - - uses: r-lib/actions/setup-r@master - with: - r-version: ${{ matrix.config.r }} - - - uses: r-lib/actions/setup-pandoc@master - - - name: Query dependencies - run: | - install.packages('remotes') - saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2) - shell: Rscript {0} - - - name: Cache R packages - if: runner.os != 'Windows' - uses: actions/cache@v1 - with: - path: ${{ env.R_LIBS_USER }} - key: ${{ runner.os }}-r-${{ matrix.config.r }}-2-${{ hashFiles('.github/depends.Rds') }} - restore-keys: ${{ runner.os }}-r-${{ matrix.config.r }}-2- + - uses: r-lib/actions/setup-pandoc@v2 - name: Install Mac system dependencies if: runner.os == 'macOS' - run: | - brew install zeromq + run: brew install zeromq - - name: Install system dependencies + - name: Install Linux system dependencies if: runner.os == 'Linux' - env: - RHUB_PLATFORM: linux-x86_64-ubuntu-gcc run: | - Rscript -e "remotes::install_github('r-hub/sysreqs')" - sysreqs=$(Rscript -e "cat(sysreqs::sysreq_commands('DESCRIPTION'))") - sudo -s eval "$sysreqs" + sudo apt-get update + sudo apt-get install libglpk-dev libglpk40 - - name: Install dependencies - run: | - remotes::install_deps(dependencies = TRUE) - remotes::install_cran("rcmdcheck") - install.packages(".", type = "source", repos = NULL) - shell: Rscript {0} + - uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ matrix.config.r }} + http-user-agent: ${{ matrix.config.http-user-agent }} + use-public-rspm: true - - name: Check - env: - _R_CHECK_CRAN_INCOMING_REMOTE_: false - _R_CHECK_FORCE_SUGGESTS_: false - run: rcmdcheck::rcmdcheck(args = c("--no-manual", "--as-cran"), error_on = "warning", check_dir = "check") - shell: Rscript {0} + - uses: r-lib/actions/setup-r-dependencies@v2 + with: + extra-packages: any::rcmdcheck + needs: check - - name: Upload check results - if: failure() - uses: actions/upload-artifact@master + - uses: r-lib/actions/check-r-package@v2 with: - name: ${{ runner.os }}-r${{ matrix.config.r }}-results - path: check + upload-snapshots: true diff --git a/.github/workflows/cover.yaml b/.github/workflows/cover.yaml index b7203f5a6..2fee64d64 100644 --- a/.github/workflows/cover.yaml +++ b/.github/workflows/cover.yaml @@ -1,58 +1,36 @@ +# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples +# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help on: [push, pull_request] name: cover jobs: cover: - runs-on: ${{ matrix.config.os }} - - name: ${{ matrix.config.os }} (${{ matrix.config.r }}) - - strategy: - fail-fast: false - matrix: - config: - - {os: ubuntu-20.04, r: 'release', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest"} - + runs-on: ubuntu-latest env: - GITHUB_PAT: ${{ secrets.GITHUBPAT }} + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + NOT_CRAN: true + R_REMOTES_NO_ERRORS_FROM_WARNINGS: false + TORCH_INSTALL: 1 + steps: - uses: actions/checkout@v2 - - uses: r-lib/actions/setup-r@master - - - uses: r-lib/actions/setup-pandoc@master - - - name: Query dependencies + - name: Install Linux system dependencies + if: runner.os == 'Linux' run: | - install.packages('remotes') - saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2) - shell: Rscript {0} + sudo apt-get update + sudo apt-get install libglpk-dev libglpk40 - - name: Cache R packages - if: runner.os != 'Windows' - uses: actions/cache@v1 + - uses: r-lib/actions/setup-r@v2 with: - path: ${{ env.R_LIBS_USER }} - key: ${{ runner.os }}-r-${{ matrix.config.r }}-4-${{ hashFiles('.github/depends.Rds') }} - restore-keys: ${{ runner.os }}-r-${{ matrix.config.r }}-4- - - - name: Install system dependencies - if: runner.os == 'Linux' - env: - RHUB_PLATFORM: linux-x86_64-ubuntu-gcc - run: | - sudo -s apt-get install libcurl4-openssl-dev - sudo -s apt-get install libzmq3-dev - sudo -s apt-get install libgit2-dev + use-public-rspm: true - - name: Install dependencies - run: | - install.packages(c("remotes", "covr")) - remotes::install_deps(dependencies = TRUE) - install.packages(".", type = "source", repos = NULL) - shell: Rscript {0} + - uses: r-lib/actions/setup-r-dependencies@v2 + with: + extra-packages: any::covr + needs: coverage - name: Test coverage - run: covr::codecov() + run: covr::codecov(quiet = FALSE) shell: Rscript {0} diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 656e590a2..3f4df0b30 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -1,44 +1,26 @@ +# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples +# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help on: [push, pull_request] name: lint jobs: lint: - runs-on: macOS-latest + runs-on: ubuntu-latest env: - GITHUB_PAT: ${{ secrets.GITHUBPAT }} + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} steps: - uses: actions/checkout@v2 - - uses: r-lib/actions/setup-r@master - - - name: Query dependencies - run: | - install.packages('remotes') - saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2) - shell: Rscript {0} - - - name: Cache R packages - uses: actions/cache@v1 + - uses: r-lib/actions/setup-r@v2 with: - path: ${{ env.R_LIBS_USER }} - key: macOS-r-4.0-3-${{ hashFiles('.github/depends.Rds') }} - restore-keys: macOS-r-4.0-3- + use-public-rspm: true - - name: Install system requirements - run: | - brew install zeromq - - - name: Install dependencies - run: | - install.packages(c("remotes")) - remotes::install_deps(dependencies = TRUE) - remotes::install_cran("lintr") - shell: Rscript {0} + - uses: r-lib/actions/setup-r-dependencies@v2 + with: + extra-packages: any::lintr, local::. + needs: lint - name: Lint - run: | - out <- lintr::lint_package() - print(out) - if (length(out)) stop("lints found") + run: lintr::lint_package() shell: Rscript {0} diff --git a/.lintr b/.lintr index 275cd3f02..15241946a 100644 --- a/.lintr +++ b/.lintr @@ -2,3 +2,4 @@ linters: linters_with_defaults( object_length_linter = NULL, object_name_linter = NULL, object_usage_linter = NULL) +exclusions: list("inst/testing/knitr/bad.Rmd") diff --git a/R/walk_code.R b/R/walk_code.R index 3df2b3e33..23d1beb48 100644 --- a/R/walk_code.R +++ b/R/walk_code.R @@ -460,8 +460,7 @@ make_assignment_fn_impl <- function(fun) { if (is_correctly_namespaced) { fun[[3]] <- as.name(paste0(as.character(fun[[3]]), "<-")) fun - } - else { + } else { stop0("bad function in complex assignments: ", dsq(fun)) } } @@ -475,8 +474,7 @@ evalseq <- function(e) { v <- evalseq(e[[2]]) e[[2]] <- codetools_tmp c(v, list(e)) - } - else { + } else { list(e) } } diff --git a/inst/rmarkdown/templates/drake/skeleton/skeleton.Rmd b/inst/rmarkdown/templates/drake/skeleton/skeleton.Rmd index 2ffdf3c68..f884ceab0 100644 --- a/inst/rmarkdown/templates/drake/skeleton/skeleton.Rmd +++ b/inst/rmarkdown/templates/drake/skeleton/skeleton.Rmd @@ -13,7 +13,7 @@ tryCatch({ print(fit) readd(hist) }, -error = function(e){ +error = function(e) { stop("please read the instructions in the R Markdown file.") }) ``` diff --git a/inst/testing/knitr/test.Rmd b/inst/testing/knitr/test.Rmd index 4f52f7c8d..7c84e4c49 100644 --- a/inst/testing/knitr/test.Rmd +++ b/inst/testing/knitr/test.Rmd @@ -45,10 +45,10 @@ f(readd(target14) + var) f(readd(target15) + var) # deliberate repeat f(g(drake::readd(target16) + var)) f(readd(target = "target17", character_only = TRUE) + var) -g <- function(){ +g <- function() { f(drake:::loadd(target18, character_only = IGNORE_THIS) + var) } -function(){ +function() { f(drake:::loadd(target18, character_only = IGNORE_THIS) + var) } readd("\"file1\"") diff --git a/tests/testthat/test-4-flow.R b/tests/testthat/test-4-flow.R index 3872c4491..629e00cc2 100644 --- a/tests/testthat/test-4-flow.R +++ b/tests/testthat/test-4-flow.R @@ -237,9 +237,13 @@ test_with_dir("failed targets do not become up to date", { test_with_dir("true targets can be functions", { skip_on_cran() # CRAN gets essential tests only (check time limits). - generator <- function() return(function(x) { - x + 1 - }) + generator <- function() { + return( + function(x) { + x + 1 + } + ) + } plan <- drake_plan(myfunction = generator(), output = myfunction(1)) make(plan, verbose = 0L, session_info = FALSE) config <- drake_config(plan, verbose = 0L, session_info = FALSE) diff --git a/tests/testthat/test-6-analysis.R b/tests/testthat/test-6-analysis.R index 1fb8df9c9..67ceddcf3 100644 --- a/tests/testthat/test-6-analysis.R +++ b/tests/testthat/test-6-analysis.R @@ -1,5 +1,6 @@ drake_context("analysis") +# nolint start test_with_dir("busy function", { f <- function(a = 1, b = k(i), nineteen, string_args = c("sa1", "sa2")) { for (iter in 1:10) { @@ -49,6 +50,7 @@ test_with_dir("busy function", { str <- sort(c(str, "w", "x", "y", "z")) expect_equal(sort(analyze_strings(f)), str) }) +# nolint end test_with_dir("equals analysis", { for (text in c("z = g(a * b)", "function(x) {z = g(a * b)}")) { @@ -143,7 +145,7 @@ test_with_dir("solitary codetools globals tests", { } expect_equivalent(drake_deps(f), new_drake_deps()) f <- function() { - x <- 1; y <- 2 + x <- 1; y <- 2 # nolint } out <- as.character(drake_deps(f)$globals) expect_equal(out, character(0)) @@ -398,9 +400,11 @@ test_with_dir("bad target names", { test_with_dir("file_in() and file_out() and knitr_in(): commands vs imports", { skip_on_cran() # CRAN gets essential tests only (check time limits). skip_if_not_installed("knitr") + # nolint start cmd <- quote({ file_in("x"); file_out("y"); knitr_in("report.Rmd") }) + # nolint end f <- function() { file_in("x") } diff --git a/tests/testthat/test-6-plans.R b/tests/testthat/test-6-plans.R index ee6b05f5b..a9cfc1d9f 100644 --- a/tests/testthat/test-6-plans.R +++ b/tests/testthat/test-6-plans.R @@ -48,7 +48,7 @@ test_with_dir("warn about <- and -> in drake_plan()", { ) expect_silent( tmp <- drake_plan( - a = 1 -> x, + a = 1 -> x, # nolint b = 2 ) ) @@ -57,11 +57,11 @@ test_with_dir("warn about <- and -> in drake_plan()", { regexp = "to assign targets to commands" ) expect_warning( - tmp <- drake_plan(a = 1, b -> 2), + tmp <- drake_plan(a = 1, b -> 2), # nolint regexp = "to assign targets to commands" ) expect_warning( - tmp <- drake_plan(a <- 1, b -> 2), + tmp <- drake_plan(a <- 1, b -> 2), # nolint regexp = "to assign targets to commands" ) }) diff --git a/tests/testthat/test-7-dsl.R b/tests/testthat/test-7-dsl.R index bcd99aac8..5a05264a2 100644 --- a/tests/testthat/test-7-dsl.R +++ b/tests/testthat/test-7-dsl.R @@ -119,7 +119,7 @@ test_with_dir("single tag_in", { .tag_in = single ) ), - trace = T + trace = TRUE ) exp <- drake_plan( x_1 = target( @@ -146,7 +146,7 @@ test_with_dir("multiple tag_in", { .tag_in = c(one, second) ) ), - trace = T + trace = TRUE ) exp <- drake_plan( x_1 = target( @@ -175,7 +175,7 @@ test_with_dir("single tag_out", { .tag_out = single ) ), - trace = T + trace = TRUE ) exp <- drake_plan( x_1 = target( @@ -202,7 +202,7 @@ test_with_dir("multiple tag_out", { .tag_out = c(one, second) ) ), - trace = T + trace = TRUE ) exp <- drake_plan( x_1 = target( diff --git a/tests/testthat/test-9-future.R b/tests/testthat/test-9-future.R index 95102d18c..d2cb59cb1 100644 --- a/tests/testthat/test-9-future.R +++ b/tests/testthat/test-9-future.R @@ -93,7 +93,7 @@ test_with_dir("future package functionality", { for (i in 1:2) { e$my_plan$command[[2]] <- as.call( c(quote(identity), quote({ - Sys.sleep(2); simulate(48) + Sys.sleep(2); simulate(48) # nolint })) ) future::plan(future::multisession)