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

Expand checking workflows to more platforms #324

Merged
merged 6 commits into from
Jun 26, 2024
Merged

Expand checking workflows to more platforms #324

merged 6 commits into from
Jun 26, 2024

Conversation

andrjohns
Copy link
Contributor

No description provided.

@jgabry
Copy link
Member

jgabry commented Jun 24, 2024

Thanks! I'll take a look at why these are failing.

@jgabry
Copy link
Member

jgabry commented Jun 24, 2024

I think there are some tiny differences in the SVG files causing failures in the visual tests (this sometimes happens when ggplot updates, although not sure in this case). I'll try updating the SVGs

@jgabry
Copy link
Member

jgabry commented Jun 24, 2024

I think a problem is that the SVGs produced for the visual tests may (sometimes) be very slightly different under different versions of R, which will be a problem for testing oldrel, release, and devel. Maybe we should just have the visual tests (a small minority of the tests, but still a large number) run on r-release and turn them of on oldrel and devel? What do you think?

@jgabry
Copy link
Member

jgabry commented Jun 24, 2024

Maybe we should just have the visual tests (a small minority of the tests, but still a large number) run on r-release and turn them of on oldrel and devel? What do you think?

@andrjohns Do you know if it's possible to detect (e.g with an environment variable or some other way) if the GHA run is using oldrel, release, or devel? I'm trying to figure out a way to detect that (so I can insert a skip_if() in the necessary tests), but so far I can only think of terrible brittle hacks or adding a dependency on the rversions package which has functions for giving you the version numbers of oldrel and release.

@jgabry
Copy link
Member

jgabry commented Jun 26, 2024

I think I can just create an environment variable in the yaml file that I can use inside testthat to just run the tests under certain versions of R. It looks like right now r-devel is failing on Mac due to an issue building the vdiffr package and not actually during visual tests. And the tests are passing on r-devel on windows and ubuntu. So for now we can just skip running the tests of the SVGs on r-oldrel. We can update in the future if devel becomes a problem too, but it would be nice to test release and devel.

Copy link
Member

@jgabry jgabry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I think this is ready to merge. I changed it so the visual tests of the SVGs using vdiffr don't run on r-oldrel, but all the other unit tests still run. For r-devel they are passing on windows and ubuntu but failing on Mac because it errors when trying to build the vdiffr package. For now I'll leave that as is in case it's a temporary issue, but we could consider disabling r-devel on mac (I noticed that the vdiffr package itself doesn't test with r-devel on mac, perhaps for the same reason? https://github.com/r-lib/vdiffr/blob/main/.github/workflows/R-CMD-check.yaml)

@jgabry jgabry merged commit 1f66c7f into master Jun 26, 2024
9 of 10 checks passed
@jgabry jgabry deleted the expand-testing branch June 26, 2024 22:03
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

Successfully merging this pull request may close these issues.

2 participants