From 850d7178e188db693aa3ca0036ac270dcca697e0 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Thu, 30 Dec 2021 23:18:52 -0600 Subject: [PATCH 1/8] [R-package] [docs] fix calculation of R test coverage (fixes #4919) --- R-package/README.md | 5 ++++- R-package/tests/testthat/test_lgb.unloader.R | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/R-package/README.md b/R-package/README.md index d610fe0410a8..dd973d2a183b 100644 --- a/R-package/README.md +++ b/R-package/README.md @@ -265,10 +265,13 @@ The example below shows how to generate code coverage for the R package on a mac ```shell # Install -sh build-cran-package.sh +sh build-cran-package.sh \ + --no-build-vignettes # Get coverage +LIGHTGBM_TEST_COVERAGE=true \ Rscript -e " \ + library(covr); coverage <- covr::package_coverage('./lightgbm_r', type = 'tests', quiet = FALSE); print(coverage); covr::report(coverage, file = file.path(getwd(), 'coverage.html'), browse = TRUE); diff --git a/R-package/tests/testthat/test_lgb.unloader.R b/R-package/tests/testthat/test_lgb.unloader.R index 6ae8ee2d35f7..17ba951e1ebc 100644 --- a/R-package/tests/testthat/test_lgb.unloader.R +++ b/R-package/tests/testthat/test_lgb.unloader.R @@ -2,7 +2,13 @@ VERBOSITY <- as.integer( Sys.getenv("LIGHTGBM_TEST_VERBOSITY", "-1") ) +CALCULATING_TEST_COVERAGE <- Sys.getenv("LIGHTGBM_TEST_COVERAGE") == "true" + test_that("lgb.unloader works as expected", { + testthat::skip_if( + condition = CALCULATING_TEST_COVERAGE + , message = "lgb.unloader() tests are skipped when calculating test coverage" + ) data(agaricus.train, package = "lightgbm") train <- agaricus.train dtrain <- lgb.Dataset(train$data, label = train$label) @@ -24,6 +30,10 @@ test_that("lgb.unloader works as expected", { }) test_that("lgb.unloader finds all boosters and removes them", { + testthat::skip_if( + condition = CALCULATING_TEST_COVERAGE + , message = "lgb.unloader() tests are skipped when calculating test coverage" + ) data(agaricus.train, package = "lightgbm") train <- agaricus.train dtrain <- lgb.Dataset(train$data, label = train$label) From ab72a414df78066488d02a6c4af5c5e4244ff914 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Sat, 1 Jan 2022 20:01:01 -0600 Subject: [PATCH 2/8] use quotes --- R-package/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R-package/README.md b/R-package/README.md index dd973d2a183b..9e59eb6848f2 100644 --- a/R-package/README.md +++ b/R-package/README.md @@ -269,7 +269,7 @@ sh build-cran-package.sh \ --no-build-vignettes # Get coverage -LIGHTGBM_TEST_COVERAGE=true \ +LIGHTGBM_TEST_COVERAGE="true" \ Rscript -e " \ library(covr); coverage <- covr::package_coverage('./lightgbm_r', type = 'tests', quiet = FALSE); From 1e1e491d6af3abbd01786309b3e99bf0e9255cc5 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Mon, 3 Jan 2022 20:18:37 -0600 Subject: [PATCH 3/8] use in_covr() instead of env variable --- R-package/README.md | 1 - R-package/tests/testthat/test_lgb.unloader.R | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/R-package/README.md b/R-package/README.md index 9e59eb6848f2..4b71ac8a660d 100644 --- a/R-package/README.md +++ b/R-package/README.md @@ -269,7 +269,6 @@ sh build-cran-package.sh \ --no-build-vignettes # Get coverage -LIGHTGBM_TEST_COVERAGE="true" \ Rscript -e " \ library(covr); coverage <- covr::package_coverage('./lightgbm_r', type = 'tests', quiet = FALSE); diff --git a/R-package/tests/testthat/test_lgb.unloader.R b/R-package/tests/testthat/test_lgb.unloader.R index 17ba951e1ebc..0db6078352f7 100644 --- a/R-package/tests/testthat/test_lgb.unloader.R +++ b/R-package/tests/testthat/test_lgb.unloader.R @@ -2,11 +2,9 @@ VERBOSITY <- as.integer( Sys.getenv("LIGHTGBM_TEST_VERBOSITY", "-1") ) -CALCULATING_TEST_COVERAGE <- Sys.getenv("LIGHTGBM_TEST_COVERAGE") == "true" - test_that("lgb.unloader works as expected", { testthat::skip_if( - condition = CALCULATING_TEST_COVERAGE + condition = covr::in_covr() , message = "lgb.unloader() tests are skipped when calculating test coverage" ) data(agaricus.train, package = "lightgbm") @@ -31,7 +29,7 @@ test_that("lgb.unloader works as expected", { test_that("lgb.unloader finds all boosters and removes them", { testthat::skip_if( - condition = CALCULATING_TEST_COVERAGE + condition = covr::in_covr() , message = "lgb.unloader() tests are skipped when calculating test coverage" ) data(agaricus.train, package = "lightgbm") From 1135d3abebfe19b0d68b770462b20642e8958ee3 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Mon, 3 Jan 2022 20:19:03 -0600 Subject: [PATCH 4/8] Update R-package/README.md Co-authored-by: Nikita Titov --- R-package/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R-package/README.md b/R-package/README.md index 4b71ac8a660d..56031f6254bc 100644 --- a/R-package/README.md +++ b/R-package/README.md @@ -271,7 +271,7 @@ sh build-cran-package.sh \ # Get coverage Rscript -e " \ library(covr); - coverage <- covr::package_coverage('./lightgbm_r', type = 'tests', quiet = FALSE); + coverage <- covr::package_coverage('./lightgbm_r', type = 'tests', quiet = FALSE); print(coverage); covr::report(coverage, file = file.path(getwd(), 'coverage.html'), browse = TRUE); " From 41e7f07566d110c86c7ddb7ff047cbc22258a298 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Mon, 3 Jan 2022 20:40:02 -0600 Subject: [PATCH 5/8] check that covr exists --- R-package/tests/testthat/test_lgb.unloader.R | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/R-package/tests/testthat/test_lgb.unloader.R b/R-package/tests/testthat/test_lgb.unloader.R index 0db6078352f7..8c6eed6f4965 100644 --- a/R-package/tests/testthat/test_lgb.unloader.R +++ b/R-package/tests/testthat/test_lgb.unloader.R @@ -3,10 +3,12 @@ VERBOSITY <- as.integer( ) test_that("lgb.unloader works as expected", { - testthat::skip_if( - condition = covr::in_covr() - , message = "lgb.unloader() tests are skipped when calculating test coverage" - ) + if (require(covr)) { # nolint + testthat::skip_if( + condition = covr::in_covr() + , message = "lgb.unloader() tests are skipped when calculating test coverage" + ) + } data(agaricus.train, package = "lightgbm") train <- agaricus.train dtrain <- lgb.Dataset(train$data, label = train$label) @@ -28,10 +30,12 @@ test_that("lgb.unloader works as expected", { }) test_that("lgb.unloader finds all boosters and removes them", { - testthat::skip_if( - condition = covr::in_covr() - , message = "lgb.unloader() tests are skipped when calculating test coverage" - ) + if (require(covr)) { # nolint + testthat::skip_if( + condition = covr::in_covr() + , message = "lgb.unloader() tests are skipped when calculating test coverage" + ) + } data(agaricus.train, package = "lightgbm") train <- agaricus.train dtrain <- lgb.Dataset(train$data, label = train$label) From 23fd47c6c298b1b9fb1f3d5e35c39bad0248a967 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Mon, 3 Jan 2022 20:58:32 -0600 Subject: [PATCH 6/8] add covr to suggests --- R-package/DESCRIPTION | 1 + 1 file changed, 1 insertion(+) diff --git a/R-package/DESCRIPTION b/R-package/DESCRIPTION index d9287584da09..0a64e246d874 100755 --- a/R-package/DESCRIPTION +++ b/R-package/DESCRIPTION @@ -48,6 +48,7 @@ NeedsCompilation: yes Biarch: true VignetteBuilder: knitr Suggests: + covr, knitr, processx, rmarkdown, From b97d37a3189b96ff2fe9b277cceda6c8e03eef2d Mon Sep 17 00:00:00 2001 From: James Lamb Date: Mon, 3 Jan 2022 22:14:53 -0600 Subject: [PATCH 7/8] use R_COVR environment variable --- R-package/DESCRIPTION | 1 - R-package/tests/testthat/test_lgb.unloader.R | 22 +++++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/R-package/DESCRIPTION b/R-package/DESCRIPTION index 0a64e246d874..d9287584da09 100755 --- a/R-package/DESCRIPTION +++ b/R-package/DESCRIPTION @@ -48,7 +48,6 @@ NeedsCompilation: yes Biarch: true VignetteBuilder: knitr Suggests: - covr, knitr, processx, rmarkdown, diff --git a/R-package/tests/testthat/test_lgb.unloader.R b/R-package/tests/testthat/test_lgb.unloader.R index 8c6eed6f4965..f2e0c101ecae 100644 --- a/R-package/tests/testthat/test_lgb.unloader.R +++ b/R-package/tests/testthat/test_lgb.unloader.R @@ -2,13 +2,13 @@ VERBOSITY <- as.integer( Sys.getenv("LIGHTGBM_TEST_VERBOSITY", "-1") ) +CALCULATING_TEST_COVERAGE <- Sys.getenv("R_COVR") == "true" + test_that("lgb.unloader works as expected", { - if (require(covr)) { # nolint - testthat::skip_if( - condition = covr::in_covr() - , message = "lgb.unloader() tests are skipped when calculating test coverage" - ) - } + testthat::skip_if( + condition = CALCULATING_TEST_COVERAGE + , message = "lgb.unloader() tests are skipped when calculating test coverage" + ) data(agaricus.train, package = "lightgbm") train <- agaricus.train dtrain <- lgb.Dataset(train$data, label = train$label) @@ -30,12 +30,10 @@ test_that("lgb.unloader works as expected", { }) test_that("lgb.unloader finds all boosters and removes them", { - if (require(covr)) { # nolint - testthat::skip_if( - condition = covr::in_covr() - , message = "lgb.unloader() tests are skipped when calculating test coverage" - ) - } + testthat::skip_if( + condition = CALCULATING_TEST_COVERAGE + , message = "lgb.unloader() tests are skipped when calculating test coverage" + ) data(agaricus.train, package = "lightgbm") train <- agaricus.train dtrain <- lgb.Dataset(train$data, label = train$label) From f80ccb059a101a3c7a201a023c1feda3e2da4750 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Tue, 4 Jan 2022 16:31:15 -0600 Subject: [PATCH 8/8] Update R-package/tests/testthat/test_lgb.unloader.R Co-authored-by: Nikita Titov --- R-package/tests/testthat/test_lgb.unloader.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R-package/tests/testthat/test_lgb.unloader.R b/R-package/tests/testthat/test_lgb.unloader.R index f2e0c101ecae..791d825d613c 100644 --- a/R-package/tests/testthat/test_lgb.unloader.R +++ b/R-package/tests/testthat/test_lgb.unloader.R @@ -2,7 +2,7 @@ VERBOSITY <- as.integer( Sys.getenv("LIGHTGBM_TEST_VERBOSITY", "-1") ) -CALCULATING_TEST_COVERAGE <- Sys.getenv("R_COVR") == "true" +CALCULATING_TEST_COVERAGE <- Sys.getenv("R_COVR", unset = "unset") != "unset" test_that("lgb.unloader works as expected", { testthat::skip_if(