Use CRAN vdiffr

[yutannihilation]

Since vdiffr 0.3.0 is released, let's

• switch vdiffr from the GitHub version to the CRAN version and (Travis already uses CRAN version of vdiffr since this commit: 9f1904d)
• rely on VDIFFR_RUN_TESTS envvar, which vdiffr checks directly, instead of USE_VDIFFR envvar

Note that it seems still better to check the version of vdiffr since we ignore the installation failure of suggested packages by _R_CHECK_FORCE_SUGGESTS_=false.

[lionel-]

It is not necessary to set NOT_CRAN because vdiffr also checks for CI, which is set on both Travis and Appveyor. Also the wrapper in helper-vdiffr might be simplified by setting VDIFFR_RUN_TESTS to override the default behaviour on Travis.

[yutannihilation]

Thanks, I need to learn about vdiffr more...

It is not necessary to set NOT_CRAN because vdiffr also checks for CI, which is set on both Travis and Appveyor.

I set NOT_CRAN because the default value of enable_vdiffr depends on it. But, it sounds like it's better to remove this line and let vdiffr to skip tests on CRAN.

ggplot2/tests/testthat/helper-vdiffr.R

Lines 1 to 2 in 2b00ea7

	# default is equal to whether NOT_CRAN is true or not
	enable_vdiffr <- identical(Sys.getenv("NOT_CRAN"), "true")

[yutannihilation]

Confirmed the visual changes are detected properly with the new settings.

https://travis-ci.org/tidyverse/ggplot2/builds/475155850
https://ci.appveyor.com/project/hadley/ggplot2/builds/21373584

[lionel-]

LGTM! But let's wait until Claus has time to take another look, since he worked on the Travis settings last time.

[clauswilke]

Should this be < "0.3.0"?

[lionel-]

I think this if can be removed entirely.

[yutannihilation]

Sure, maybe I was too defensive.

[clauswilke]

Looks good to me to.

@lionel- When vdiffr is disabled, does expect_doppelganger() still execute the plotting code, but just doesn't compare svgs, or does it not even build the plot? I think it would be good to always build the plot, even if svg comparison is switched off.

[lionel-]

Yup that's how it works. The SVGs are always generated no matter what, so we still get hard failures when the plot doesn't build properly, and we can still use expect_warning() when we expect plot building to warn.

[yutannihilation]

I removed the if statement, which is not really necessary for now; I added the if for the case when we would come to want to enable vdiffr for older versions of R, where the installation failures of suggested package are ignored (_R_CHECK_FORCE_SUGGESTS_=false) so that the older version of vdiffr might be used. But, this seems unlikely to occur.

[yutannihilation]

Thanks for reviewing!

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/