r-lib / vdiffr

Visual regression testing and graphical diffing with testthat
https://vdiffr.r-lib.org
Other
185 stars 31 forks source link

using `vdiffr` to account for slight svg changes on different OS #138

Closed jtlandis closed 7 months ago

jtlandis commented 9 months ago

Hello, thank you to the maintainers and contributors that creating a wonderful package. I was hoping we could possible start a discussion regarding best practices in using this package, specifically expect_doppelganger() and it consistently failing on a particular OS environment. In my use case, I am using github actions to automatically run R CMD check for various OS environments:

config:
          - {os: macOS-latest,   r: 'release'}
          - {os: windows-latest, r: 'release'}
          - {os: ubuntu-latest,   r: 'devel', http-user-agent: 'release'} #the problem environment
          - {os: ubuntu-latest,   r: 'release'}
          - {os: ubuntu-latest,   r: 'oldrel-1'}

Lately, only the ubuntu-latest, r:'devel' environment is failing consistently, and after downloading the output and reviewing the differences with testthat::snapshot_review() I see that the differences are very minuscule, and often related to how paths were drawn, not obvious to the human eye.

My concern here is largely due CRAN's automated checks. Would this be one of those things in which it would be recommended to skip those specific tests with testthat:skip_on_cran(), at least until the CI environments start to pass again?

Or is there a way to have the vdiffr::expect_doppelganger() make new snapshot svg files for specific OS environments and r version combinations?

Any advice is greatly appreciated

IndrajeetPatil commented 9 months ago

Skipping on CRAN

Note that the graphical snapshot tests from {vdiffr} are skipped by default, and so you don't need to add skips for {testhat} unless you've overridden this argument:

Screenshot 2023-12-16 at 16 45 27

OS-specific failures

This is definitely a trickier issue to deal with[^1]:

AFAICT, {vdiffr} currently doesn't allow a straightforward API to create variants of snapshots, and it would indeed be a nice addition.

[^1]: I've more detailed thoughts on snapshot testing in these slides, in case you are interested.