Skip to content

Visual tests don't seem to run on Travis #2838

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

Closed
clauswilke opened this issue Aug 18, 2018 · 5 comments
Closed

Visual tests don't seem to run on Travis #2838

clauswilke opened this issue Aug 18, 2018 · 5 comments

Comments

@clauswilke
Copy link
Member

From all I can tell, visual tests don't run on Travis. I have three lines of evidence:

  1. In PR make preserve = "single" more robust in position_dodge(). #2813, I ran a build with and without a change in the visual test reference images. Both builds succeeded on Travis. Only one succeeded on my local install.

  2. Numerous visual tests seem to have changed with the latest version of scales (Visual tests fail with scales 1.0 #2824), but no Travis build fails because of this.

  3. I can't remember when was the last time that I had a Travis build fail because of a failed visual test. Yet in my local install it's quite common that visual tests fail on occasion, and generally for valid reasons (something small has changed in the plot).

cc @lionel-

@clauswilke
Copy link
Member Author

@lionel- I think I have figured out the problem. I believe it is in these lines in the vdiffr code:
https://github.com/lionel-/vdiffr/blob/43fbfce764b2c99960614ebc7472dca190fda3e6/R/testthat-ui.R#L53-L57

This code causes vdiffr to silently (from within CRAN check) skip tests when the system freetype library is too old. The freetype version on Travis is 2.5.2, hence that condition is satisfied. On our local machines we have more recent versions of freetype and thus we never notice the problem.

I have made a PR to test this hypothesis for the ggplot2 repo (#2841), but I've seen the same problem already in one of my own repos: https://travis-ci.org/wilkelab/cowplot/builds/417734551

I think the freetype version test in vdiffr should be removed since vdiffr now uses a defined version anyways.

This raises another issue, though: If vdiffr for whatever reason decides to skip tests while on Travis then we don't get notified and may assume everything is fine when actually nothing is tested. Maybe there needs to be a strict mode that treats skipped tests as errors, and that mode should then be enabled on Travis.

@clauswilke
Copy link
Member Author

As expected (https://travis-ci.org/tidyverse/ggplot2/jobs/417736237):

── 1. Error: aesthetics are drawn correctly (@test-aes.r#120)  ─────────────────
lib freetype too old: 2.5.2
1: expect_doppelganger("stat='identity'", ggplot(dat, aes(x = xvar, y = yvar)) + geom_bar(stat = "identity")) at testthat/test-aes.r:120
2: stop("lib freetype too old: ", ver) at /home/travis/build/tidyverse/ggplot2/ggplot2.Rcheck/tests/testthat/helper-vdiffr.R:20

@lionel-
Copy link
Member

lionel- commented Aug 19, 2018

Thanks to your help diagnosing the vdiffr issue the tests are now run again on Travis @clauswilke!

Unfortunately it seems many visual tests have not been updated because of the silent skipping. As I'm not familiar with all the recent changes I'm not sure whether the failures reflect actual bugs or (more likely) figures that should simply be updated. Could you take a look based on current master please?

@clauswilke
Copy link
Member Author

@lionel- Thanks for taking care of this so quickly!

I'm aware of the broken visual tests, that's what prompted me to look into Travis in the first place. I opened an issue a few days ago: #2824

The broken tests are due to changes in secondary axis code (those are all fine) and changes in the scales package (those are sufficiently different that I don't feel comfortable to approve without a second set of eyes). I think @hadley and/or @dpseidel need to take a look. Issue #2824 contains a few example images.

@lock
Copy link

lock bot commented Feb 16, 2019

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/

@lock lock bot locked and limited conversation to collaborators Feb 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants