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.

Sign up for GitHub

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

Jump to bottom

{usethis} should move to Imports rather than Suggests? #193

Closed
daattali opened this issue May 2, 2022 · 4 comments · Fixed by #199
Closed

{usethis} should move to Imports rather than Suggests? #193

daattali opened this issue May 2, 2022 · 4 comments · Fixed by #199
Labels
bug Something isn't working
Milestone
v0.2.0

Comments

@daattali
Copy link
Contributor

daattali commented May 2, 2022

I just tried installing {shinytest2} in a brand new environment. Running shinytest2::record_test() was working fine all the way until the end when I clicked the "Save test and exit" button. Clicking that button resulted in an error in the console:

Error in write_union("tests/testthat/setup.R", comments = "# Load application support files into testing environment",  : 
  The package `usethis` is required.

To me this suggests that {usethis} is an essential package and should be automatically installed when {shinytest2} is installed. Since record_test() is the recommended way of using this package, that function should work out of the box.

Feel free to close this issue if you disagree :)
@schloerke
Copy link
Collaborator

Yes! I agree.

The logic had changed since adding usethis

@schloerke schloerke added this to the v0.2.0 milestone May 3, 2022
@schloerke
Copy link
Collaborator

Going to adjust internal logic to not require usethis for recordings.

-> Going to keep usethis as Suggests.

@schloerke schloerke added the bug Something isn't working label May 3, 2022
@daattali
Copy link
Contributor Author

daattali commented May 3, 2022

I don't want to open a new issue, so I'll mention here:
Some other functions mentioned in the Getting Started page also don't work without installing packages. I remember trying to blindly follow the docs and getting scared when things weren't working and I was getting errors. I believe it was vdiffr and showimage. Perhaps when saying to look at the differences using snapshot_review(), adding a short phrase like "(you'll need to have the X package installed)" will be good

@schloerke
Copy link
Collaborator

( Moved to new issue #198 )

schloerke added a commit that referenced this issue May 3, 2022
@schloerke
Adjust logic for write_union to not require usethis
7a18f80 
Fixes #193
@schloerke schloerke mentioned this issue May 3, 2022
Merged
1 task
schloerke added a commit that referenced this issue May 3, 2022
@schloerke
Post release bugs found by Dean (#199)
c29b78e 
* Adjust logic for `write_union` to not require usethis

Fixes #193

* Make sure test file opens in RStudio when done

Fixes #194

* Disable dev website. Changes need to be made now

* Make sure images are always included

Fixes #195

* `$get_screenshot()` is displayed in the graphics device or plots tab

Fixes #196

* Safe guard vdiffr

* Mention installing `{diffviewer}`

Fixes #198

* Rename `app_screenshot()` -> `app_get_screenshot()`

* Update NEWS.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
v0.2.0
Development

Successfully merging a pull request may close this issue.

Post release bugs found by Dean rstudio/shinytest2
2 participants
@schloerke @daattali