-
Notifications
You must be signed in to change notification settings - Fork 33
Add github actions to run tests too slow for CRAN #17
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
base: master
Are you sure you want to change the base?
Conversation
e741c0d to
d9d3378
Compare
nikola-rados
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good @corviday, nice work! I have some questions and some formatting suggestions but overall this does the trick!
| # this action runs the same checks on the repository that CRAN does: | ||
| # 1. windows container with latest R release | ||
| # 2. linux container with latest R release | ||
| # 3. linux container with next R release | ||
| # to offer a preview of whether changes to the package will be | ||
| # accepted by CRAN. Testing on Mac is not done because there is not | ||
| # udunits2 library for Mac, as far as I can tell. | ||
| # It also runs the tests that are too slow for CRAN. These extra tests | ||
| # are in the tests/extra-tests directory. | ||
| # it is derived from the check-standard action from the r-lib repository, | ||
| # which can be seen here: | ||
| # https://github.com/r-lib/actions/blob/master/examples/check-standard.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment here!
| check: | ||
| runs-on: ${{ matrix.config.os }} | ||
|
|
||
| name: ${{ matrix.config.os }} (${{ matrix.config.r }}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly are we naming here? The different jobs within the steps?
| fail-fast: false | ||
| matrix: | ||
| config: | ||
| - {os: windows-latest, r: 'release'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of latest could we specify the version here? I just don't want this to break in the future due to a version change.
| - uses: actions/checkout@v2 | ||
|
|
||
| - uses: r-lib/actions/setup-r@master | ||
| with: | ||
| r-version: ${{ matrix.config.r }} | ||
|
|
||
| - uses: r-lib/actions/setup-pandoc@master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For styling purposes could we remove the extra newlines in here?
| sudo -s eval "$sysreqs" | ||
| - name: Install dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throw a newline between these separate name sections.
| _R_CHECK_CRAN_INCOMING_REMOTE_: false | ||
| run: rcmdcheck::rcmdcheck(args = c("--no-manual", "--as-cran"), error_on = "warning", check_dir = "check") | ||
| shell: Rscript {0} | ||
| - name: Upload check results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
| if: failure() | ||
| uses: actions/upload-artifact@master | ||
| with: | ||
| name: ${{ runner.os }}-r${{ matrix.config.r }}-results | ||
| path: check No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if there is a failure during this testing why do we need to upload the results? Wouldn't the failure info be in the actions logs? If we are not actively using these logs we could leave out this section.
| @@ -0,0 +1,99 @@ | |||
| ## [1.0.8] - 2021-04-26 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, a changelog is always helpful!
| person("Cairo Sanders", role = "ctb", email = "cairosanders@uvic.ca"), | ||
| person("Arelia Schoeneberg", role = "aut", email = "wernera@uvic.ca"), | ||
| person("Stephen Sobie", role = "aut", email = "ssobie@uvic.ca"), | ||
| person("Lee Zeman", role = c("aut","cre"), email = "lzeman@uvic.ca")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like some of the formatting here is a bit off, will this have any kind of impact on the build?
| install.packages('remotes') | ||
| saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2) | ||
| writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version") | ||
| shell: Rscript {0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, but why don't we just run this in run?
This PR adds automatic testing with github actions to this repository.
There are two goals for the CI:
R CMD check --as-crancommand builds the code, checks it for various issues CRAN cares about (including documentation - we have to build documentation first), and runs all the tests in thetestsdirectory. This command is run inside three containers that match the ones used by CRAN to test uploads: a linux container running the latest formally released version of R, a linux container running the bleeding-edge version of R, and a windows container running the latest formally released version of R. It should be pretty much what CRAN does - if these tests pass, CRAN should accept the package.tests/extra-testsfolder in the repository and on CRAN, but during CI, they are copied to the maintestsfolder and run byR CMD check --as-cranwith the rest of the tests.It might be nice to include some tests on a Mac container, but as far as I can tell, Macs can't install the prerequisite udunits2 library.