Migrate CI from travis to github actions?

[russHyde]

Would it be a good idea to move away from travis CI, and use GHA instead? It doesn't look like free open source support can be guaranteed at travis for the foreseeable future.

I've noted a couple of lintr users have raised issues re using lintr on GHA, but i haven't any experience using GHA yet.

[dragosmg]

I could try to help with this.

[russHyde]

Cool. Do you have any experience setting up github actions for R packages?

If not, @jimhester did a talk at rstudio conf on this topic earlier in the year: https://www.jimhester.com/talk/2020-rsc-github-actions/

At present our CI runs against a few versions of r (doing R CMD check). It also does a test coverage analysis.

[dragosmg]

Yep, I have set up GitHub CI for a number of R packages already. I am tempted to go the {tidyverse} way, using usethis::use_github_action_check_full().

This action installs the last 5 minor R versions and runs R CMD check via the rcmdcheck package on the three major OSs (linux, macOS and Windows). This action is what the tidyverse teams uses on their repositories, but is overkill for less widely used packages, which are better off using the simpler use_github_action_check_release().

I'll try to submit a PR and include test coverage too.

Update: {vctrs} has opted for 2 separate workflows - one to run the R CMD check and the other one to run test coverage.

[dragosmg]

@russHyde I think I managed to configure CI to run correctly with GitHub Actions. Please have a look at the results - https://github.com/dragosmg/lintr/actions. Some tests are failing on Windows, but all tests pass on macOS and Ubuntu.
These are the test failures on Windows:

== testthat results ===========================================================
FAILURE (test-get_source_expressions.R:75:3): Terminal newlines are detected correctly
FAILURE (test-methods.R:14:3): it returns the input trimmed to the last full lint if one exists within the max
FAILURE (test-methods.R:16:3): it returns the input trimmed to the last full lint if one exists within the max
FAILURE (test-methods.R:18:3): it returns the input trimmed to the last full lint if one exists within the max

Travis CI checks against Linux only so not sure how much of a concern this is.

Any thoughts?

[AshesITR]

I can repro the last three errors sporadically on my Windows machine.
Would be nice to find their root cause.

The first one is a fairly recent test and shouldn't fail.

[dragosmg]

I don't have access to a Windows machine and cannot reproduce any of the failures. I see 2 options going forward:

1. remove windows checks from the GitHub Actions workflow matrix
   • create an investigation issue
   • solve the problem(s)
   • switch Windows checks back on
2. solve Windows failures before merging the GitHub Actions CI PR
3. (later addition) skip_on_ci() for the failing tests.

I'd be tempted to suggest approach 1 since it solves this issue (migrates to GH Actions and adds checks on more OSs than at present), but it would definitely be nice to have all tests passing on all major platforms.

[AshesITR]

I've looked into the failure and to me it seems, the test case is likely wrong:

> substr(t1, 1, 195)
[1] "[R/AES.R:30:34:](https://github.com///blob//R/AES.R#L30) *style:* **Put spaces around all infix operators.**\r\n```r\r\n            for (i in seq_len(len/16)) {\r\n                                ~^~\r\n"
> trim_output(t1, max = 200)
[1] "[R/AES.R:30:34:](https://github.com///blob//R/AES.R#L30) *style:* **Put spaces around all infix operators.**\r\n```r\r\n            for (i in seq_len(len/16)) {\r\n                                ~^~\r\n```\r\n"

Note that all lints in the file follow the pattern

1. Lint info (file, line, link, message)
2. R code fence (```r)
3. Offending R code
4. Lint location marker
5. Closing code fence (```)

The expected result is missing the closing code fence so to me it seems the test case as well as the implementation on macOS and linux are bugged...

@jimhester can you see what's correct?

[AshesITR]

Re the other test case: fixed in #576.
It was caused by windows newlines being two bytes instead of one so shaving off one byte from the end didn't completely remove the trailing newline.

[AshesITR]

@dragosmg I just noticed the other test failure has the same root cause (\r\n vs \n).
#576 will also fix that test now, so option 1. is basically ready once it's merged.

[dragosmg]

@AshesITR cool. sounds good. I'll pull main once #576 gets merged and then finalise the PR.

[dragosmg]

@dragosmg I just noticed the other test failure has the same root cause (\r\n vs \n).
#576 will also fix that test now, so option 1. is basically ready once it's merged.

CI on multiple OSs already paying off. 😉

[russHyde]

Thanks so much for looking into this @dragosmg . That the CI runs complete on the same day is a bonus, the Travis builds seem to be taking for ever at the moment.

[dragosmg]

I hadn't realised Travis has got so bad lately. Most checks should finish in under 10 minutes on GitHub CI. There is a caching step that speeds up the installation of dependencies.

Many thanks to @jimhester and the other developers who made this functionality super easy to use with {usethis}.