From 35f608265b21d5ba65409affe04f8956d0e34b92 Mon Sep 17 00:00:00 2001 From: Gregor Gorjanc Date: Tue, 10 Feb 2026 21:34:35 +0000 Subject: [PATCH] Fix jarl complaints and add pre-commit to GitHub actions --- .github/workflows/pre-commit.yaml | 45 +++++++++++++++++++ README.md | 12 ++--- .../tests/testthat/test_TableCollection.R | 3 ++ .../testthat/test_load_summary_and_dump.R | 6 +++ RcppTskit/tests/testthat/test_misc.R | 5 +++ .../tests/testthat/test_r_to_py_and_py_to_r.R | 4 ++ 6 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/pre-commit.yaml diff --git a/.github/workflows/pre-commit.yaml b/.github/workflows/pre-commit.yaml new file mode 100644 index 0000000..ac830de --- /dev/null +++ b/.github/workflows/pre-commit.yaml @@ -0,0 +1,45 @@ +on: + push: + branches: [main, devel] + pull_request: + +name: pre-commit.yaml + +permissions: read-all + +jobs: + pre-commit: + runs-on: ubuntu-latest + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - uses: r-lib/actions/setup-r@v2 + with: + use-public-rspm: true + + - name: Install Rcpp + run: Rscript -e 'install.packages("Rcpp")' + + - name: Install system dependencies + run: | + sudo apt-get update + sudo apt-get install --yes clang-format clang-tidy + + - name: Install Air + uses: posit-dev/setup-air@v1 + + - name: Install Jarl + run: | + curl --proto '=https' --tlsv1.2 -LsSf \ + https://github.com/etiennebacher/jarl/releases/latest/download/jarl-installer.sh | sh + echo "$HOME/.local/bin" >> "$GITHUB_PATH" + + - uses: pre-commit/action@v3.0.1 + with: + extra_args: --all-files --show-diff-on-failure diff --git a/README.md b/README.md index e6b997c..1be0af9 100644 --- a/README.md +++ b/README.md @@ -197,11 +197,13 @@ on each push and pull request. Specifically, we run: * [R CMD check](.github/workflows/R-CMD-check.yaml) on multiple platforms (see curent status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/R-CMD-check.yaml)), -* [covr test coverage](.github/workflows/covr.yaml) - (see current status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/test-coverage.yaml)), and -* [Roxygen documentation generation](.github/workflows/document.yaml) - (see current status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/document.yaml)). +* [documentation generation](.github/workflows/document.yaml) + (see current status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/document.yaml)), +* [pre-commit hooks](.github/workflows/pre-commit.yaml) + (see current status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/pre-commit.yaml)), and +* [test coverage](.github/workflows/test-coverage.yaml) + (see current status [here](https://github.com/HighlanderLab/RcppTskit/actions/workflows/test-coverage.yaml)). [R universe for RcppTskit](https://highlanderlab.r-universe.dev/RcppTskit) also provides another set of checks - see [here](https://highlanderlab.r-universe.dev/RcppTskit#checktable). -These are provided once the code is merged into this repository. +These are provided after new code is pushed or merged into this repository. diff --git a/RcppTskit/tests/testthat/test_TableCollection.R b/RcppTskit/tests/testthat/test_TableCollection.R index 42748e3..b077cea 100644 --- a/RcppTskit/tests/testthat/test_TableCollection.R +++ b/RcppTskit/tests/testthat/test_TableCollection.R @@ -131,6 +131,7 @@ test_that("TableCollection and TreeSequence round-trip works", { tc <- ts$dump_tables() expect_true(is(tc, "TableCollection")) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(p <- tc$print()) expect_equal( p, @@ -177,7 +178,9 @@ test_that("TableCollection and TreeSequence round-trip works", { ts2 <- tc$tree_sequence() expect_true(is(ts2, "TreeSequence")) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(ts_print <- ts$print()) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(ts2_print <- ts2$print()) expect_equal(ts_print, ts2_print) diff --git a/RcppTskit/tests/testthat/test_load_summary_and_dump.R b/RcppTskit/tests/testthat/test_load_summary_and_dump.R index 9c77207..40bac75 100644 --- a/RcppTskit/tests/testthat/test_load_summary_and_dump.R +++ b/RcppTskit/tests/testthat/test_load_summary_and_dump.R @@ -13,6 +13,7 @@ test_that("ts/tc_load(), ts/tc_summary*(), and ts/tc_dump(x) work", { ) expect_no_error(tc_load(ts_file, skip_tables = TRUE)) check_empty_tables <- function(ts) { + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(p <- ts$print()) expect_true(all(p$tables$number == 0)) } @@ -50,6 +51,7 @@ test_that("ts/tc_load(), ts/tc_summary*(), and ts/tc_dump(x) work", { ) expect_no_error(tc_load(ts_file, skip_tables = TRUE)) check_empty_tables <- function(tc) { + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(p <- tc$print()) expect_true(all(p$tables$number == 0)) } @@ -225,6 +227,7 @@ test_that("ts/tc_load(), ts/tc_summary*(), and ts/tc_dump(x) work", { regexp = "ts must be an object of externalptr class!" ) p_ptr <- ts_ptr_print(ts_ptr) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(p <- ts$print()) expect_equal( p, @@ -276,6 +279,7 @@ test_that("ts/tc_load(), ts/tc_summary*(), and ts/tc_dump(x) work", { regexp = "tc must be an object of externalptr class!" ) p_ptr <- tc_ptr_print(tc_ptr) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(p <- tc$print()) expect_equal( p, @@ -549,6 +553,7 @@ test_that("ts/tc_load(), ts/tc_summary*(), and ts/tc_dump(x) work", { expect_equal(m_ptr, m) p_ptr <- ts_ptr_print(ts_ptr) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(p <- ts$print()) expect_equal( p_ptr, @@ -630,6 +635,7 @@ test_that("ts/tc_load(), ts/tc_summary*(), and ts/tc_dump(x) work", { expect_equal(m_ptr_tc, m_ptr_ts) p_ptr <- tc_ptr_print(tc_ptr) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(p <- tc$print()) expect_equal( p_ptr, diff --git a/RcppTskit/tests/testthat/test_misc.R b/RcppTskit/tests/testthat/test_misc.R index 0204ea9..4544bb6 100644 --- a/RcppTskit/tests/testthat/test_misc.R +++ b/RcppTskit/tests/testthat/test_misc.R @@ -11,13 +11,18 @@ test_that("tskit_version() works", { }) test_that("tsk_bug_assert() works", { + # jarl-ignore internal_function: it's just a test expect_error(RcppTskit:::test_tsk_bug_assert_c()) + # jarl-ignore internal_function: it's just a test expect_error(RcppTskit:::test_tsk_bug_assert_cpp()) }) test_that("tsk_trace_error() works", { t <- "You have to compile with -DTSK_TRACE_ERRORS to run these tests. See src/Makevars.in." + # jarl-ignore internal_function: it's just a test skip_if_not(RcppTskit:::tsk_trace_errors_defined(), t) + # jarl-ignore internal_function: it's just a test expect_warning(RcppTskit:::test_tsk_trace_error_c()) + # jarl-ignore internal_function: it's just a test expect_warning(RcppTskit:::test_tsk_trace_error_cpp()) }) diff --git a/RcppTskit/tests/testthat/test_r_to_py_and_py_to_r.R b/RcppTskit/tests/testthat/test_r_to_py_and_py_to_r.R index b54e54a..7af9762 100644 --- a/RcppTskit/tests/testthat/test_r_to_py_and_py_to_r.R +++ b/RcppTskit/tests/testthat/test_r_to_py_and_py_to_r.R @@ -142,7 +142,9 @@ test_that("ts_r_to_py() and ts_py_to_r() work", { expect_equal(length(ts2_py$tables$mutations$metadata), m2$mutations) expect_true(is(ts2_r, "TreeSequence")) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(ts2_r_print <- ts2_r$print()) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(ts_ptr2_r_print <- ts_ptr_print(ts_ptr2_r)) expect_equal(ts2_r_print, ts_ptr2_r_print) }) @@ -264,7 +266,9 @@ test_that("tc_r_to_py() and tc_py_to_r() work", { expect_equal(length(tc2_py$mutations$metadata), m2$mutations) expect_true(is(tc2_r, "TableCollection")) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(tc2_r_print <- tc2_r$print()) + # jarl-ignore implicit_assignment: it's just a test tmp <- capture.output(tc_ptr2_r <- tc_ptr_print(tc_ptr2_r)) expect_equal(tc2_r_print, tc_ptr2_r) })