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

Adjust test to only check for added dependency #211

Merged
merged 1 commit into from
Sep 15, 2021

Conversation

schloerke
Copy link
Contributor

@schloerke schloerke commented Sep 14, 2021

...not test the number of all dependencies found.


Shiny is changing the number of dependencies being returned. This test breaks when the new version of Shiny is being reverse dependency checked by CRAN.

Once merged, is it possible to release shinyMobile to CRAN? (This PR is a blocker for the release of the new version of Shiny.) Please let me know if I can help in the release process.

Thank you!
Barret

... not what shiny's dependencies are
@wch
Copy link

wch commented Sep 14, 2021

For a little context, we submitted a new version of Shiny to CRAN, and it was rejected in part because of the failing test in shinyMobile. If you could merge and make a release as soon as possible, we would appreciate it! That will allow us to release Shiny 1.7.0.

This is part of the email we got from CRAN about the downstream test failure:

> Package: shinyMobile
> Check: tests
> New result: ERROR
>      Running ‘testthat.R’ [1s/1s]
>    Running the tests in ‘tests/testthat.R’ failed.
>    Complete output:
>      > library(testthat)
>      > library(shinyMobile)
>      >
>      > test_check("shinyMobile")
>      â• â•  Failed tests â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â• â•
>      ── Failure (test-preview_mobile.R:25:3): dependencies ──────────────────────────
>      `deps` has length 3, not length 2.
>     
>      [ FAIL 1 | WARN 0 | SKIP 0 | PASS 72 ]
>      Error: Test failures
>      Execution halted

@DivadNojnarg
Copy link
Member

Thanks for reporting.

This is not the first time this one fails cc @cpsievert. Could you mail me next time so that I can run the test with new Shiny version before you submit to CRAN? This is quick and easy to check so I don't mind and it will avoid you to wait.

@wch
Copy link

wch commented Sep 15, 2021

Normally we run R CMD check on all packages that depend on Shiny, using revdepcheck, and that would report errors in downstream packages like shinyMobile. Then we would email package maintainers about the problems a couple of weeks before we release Shiny. This time, however, we had technical problems, and our revdepcheck results didn't report the error in shinyMobile.

I would suggest that, when possible, try to write tests that are less specific about the exact data structures returned from Shiny, since they are subject to change over time.

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.

3 participants