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.

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

CRAN feedback on package checking time #6400

Open
TysonStanley opened this issue Aug 26, 2024 · 8 comments
Open

CRAN feedback on package checking time #6400

TysonStanley opened this issue Aug 26, 2024 · 8 comments
Milestone

Comments

@TysonStanley
Copy link
Member

During package submission process, Uwe said:

need to reduce the check time, this time we see * checking tests ... [16m] OK which is really too much as the overall check time of a package should be less than 10 min and this may be killed during the checks.

IIt's much shorter on my machine (macOS) and from what I could see, this seemed to be on Windows (from my testing of the package before submission using WinBuilder). Regardless, something we need to include in our next submission to CRAN (likely patch).

@MichaelChirico
Copy link
Member

Frustrating feedback. Checks run on windows GHA in 5 minutes. I don't understand why CRAN would encourage worse package testing like this -- especially from a package like {data.table} that has 1500 reverse dependencies / 3,000 recursive revdeps. This is how we wind up with packages like "IF (on CRAN) dont_test()" -- really bad for the ecosystem overall.

Does anybody even have a machine that can reproduce these terrible times?

@TysonStanley
Copy link
Member Author

Agreed, strange incentives. But no I can't replicate other than when I use WinBuilder (which I assume is what they use for CRAN checks too).

@MichaelChirico
Copy link
Member

other than when I use WinBuilder

Well, that's something at least. Not the friendliest feedback mechanism, but there is a mechanism...

@MichaelChirico MichaelChirico added this to the 1.17.0 milestone Aug 26, 2024
@ben-schwen
Copy link
Member

What about deactivating only the most time consuming tests on CRAN?

@MichaelChirico
Copy link
Member

Once we open the pandora's box of "running a different suite of tests on CRAN", there's a lot of options open to us. That one probably makes the most sense, though I'm not sure how confident we are about which are the slowest tests on the CRAN machines.

@eddelbuettel
Copy link
Contributor

eddelbuettel commented Aug 27, 2024

Once we open the pandora's box of "running a different suite of tests on CRAN", there's a lot of options open to us.

FWIW Rcpp has done that for maybe a decade and is still alive. The package looks at the version, and when it sees a form 'a.b.c' runs reduced / quick tests -- which is what CRAN does -- whereas developers, GitHub Actions, r-universe, ... see a.b.c.d (which is usually set during development cycles) and run the full tests. You could even only skip on Windows. It's all programmable after all -- and CRAN has to look after 21k packages on (essentially) one honking big machine for each architecture so you may not have the machine to yourself.

@TysonStanley
Copy link
Member Author

Quick glance at Rcpp @eddelbuettel, do you use the version number to define when tests would be reduced?

@eddelbuettel
Copy link
Contributor

eddelbuettel commented Aug 29, 2024

Yes -- It's an old scheme from a time when IIRC the time limits at CRAN were harsher and we may have had issues on either windoze or slowlaris so there was an added incentive to limit things. It has stood the test of time.

As you know, under R all tests start by entry points in tests/, by convention via the name of test runner package (or containing it -- doRUnit.R). So I place the check there, and I (happily) use tinytests it is in tests/tinytest.R:

https://github.com/RcppCore/Rcpp/blob/1e82f1872eebe4fc813161ec0bac9bdfc53e1619/tests/tinytest.R#L10-L21

    ## Force tests to be executed if in dev release which we define as
    ## having a sub-release, eg 0.9.15.5 is one whereas 0.9.16 is not
    if (length(strsplit(packageDescription("Rcpp")$Version, "\\.")[[1]]) > 3) {	# dev rel, and
        if (Sys.getenv("RunAllRcppTests") != "no") { 			# if env.var not yet set
            message("Setting \"RunAllRcppTests\"=\"yes\" for development release\n")
            Sys.setenv("RunAllRcppTests"="yes")
        }
        if (Sys.getenv("RunVerboseRcppTests") != "no") { 		# if env.var not yet set
            message("Setting \"RunVerboseRcppTests\"=\"yes\" for development release\n")
            Sys.setenv("RunVerboseRcppTests"="yes")
        }
    }

(We have a second condition here to also suppress tests (that build packages using Rcpp etc) which mess stdout up, but that is not material here.)

We communicate the status via an environment variable. As (thankfully !!) under tinytest each test file is a self-contained R script we elect to skip there if the env var is not set. From the lexiocigraphically first:

https://github.com/RcppCore/Rcpp/blob/1e82f1872eebe4fc813161ec0bac9bdfc53e1619/inst/tinytest/test_algorithm.R#L19

if (Sys.getenv("RunAllRcppTests") != "yes") exit_file("Set 'RunAllRcppTests' to 'yes' to run.")

And that is all that there is to it. Because it is an envionment variable you can also skip sections of a file via simple if (...). We use a similar scheme relying on an external environment like CI to dial some tests up and others down for another large package at work, but that is still a non-CRAN package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants