single-cell-data / TileDB-SOMA

Python and R SOMA APIs using TileDB’s cloud-native format. Ideal for single-cell data at any scale.
https://tiledbsoma.readthedocs.io
MIT License
90 stars 25 forks source link

[R] Move failing R CI workflow to daily cron #2922

Closed ryan-williams closed 1 month ago

ryan-williams commented 1 month ago

Issue and/or context: #2906

Changes: Move r-ci.yml to a daily cron, to avoid ❌'s on PRs / main / releases.

eddelbuettel commented 1 month ago

Maybe we can consider the simpler alternative of doing what macOS and coverage do: reduced tests. We can set CI="" here:

https://github.com/single-cell-data/TileDB-SOMA/blob/6f88d577c6e73945824e23104874717d21078898/.github/workflows/r-ci.yml#L153

or equivalently change the definition of when extended tests run in

https://github.com/single-cell-data/TileDB-SOMA/blob/6f88d577c6e73945824e23104874717d21078898/apis/r/tests/testthat/helper-test-tiledb-objects.R#L14-L25

where the simplest change would just be to return(FALSE) at the top.

ryan-williams commented 1 month ago

Agreed that excluding only the known failing tests, and continuing to run others, is preferable. I can try to make the changes @eddelbuettel suggested above this afternoon. I am also happy for someone more familiar with the R side to make those changes (on this PR, or a separate one).

Continuing to run the failing tests on a cron schedule could make sense in addition to the above, unless we are not interested in ever tracking those failures.

eddelbuettel commented 1 month ago

Yes, doing both is not a bad idea. To reduce the fail can you please try the following

modified   .github/workflows/r-ci.yml
@@ -150,7 +150,7 @@ jobs:

       - name: Test
         if: ${{ matrix.covr == 'no' }}
-        run: cd apis/r/tests && Rscript testthat.R
+        run: cd apis/r/tests && CI="" Rscript testthat.R

       - name: Coverage

Happy to do it too, of course.

PS For a 'both' approach the above has to be a variable or tested if invoked by schedule.

ryan-williams commented 1 month ago

Per https://github.com/single-cell-data/TileDB-SOMA/issues/2906#issuecomment-2318012637, R CI is mostly passing since #2926 (though still segfaulting ≈10-20% of the time).

One of the changes @eddelbuettel suggested above might still make sense, but switching the whole CI to cron does not, so I'm going to close this.