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

Update UI test expectations to accommodate shiny 1.6 #38

Merged
merged 1 commit into from
Jan 11, 2021

Conversation

cpsievert
Copy link

@cpsievert cpsievert commented Jan 10, 2021

In the next version of shiny (which we plan on submitting to CRAN in a week or so), we've made several changes to HTML/CSS markup that cause your unit tests to fail (in non-meaningful ways), and so we ask you to submit a new version of cytomapper to CRAN as soon as possible to address this issue.

This PR fixes the issue by updating your test expectations to work with shiny 1.6 and also adds skip_on_cran() to skip tests that make particularly strong assumptions about the HTML/CSS that shiny generates. This will make it so shiny can continue to make HTML/CSS changes as needed without requiring periscope to send updates to CRAN (you can still monitor the tests locally and/or on a CI service like Github Actions or Travis CI).

Let me know if you have any questions and please send a new version of periscope to CRAN with these updates to your tests as soon as possible.

@codecov
Copy link

codecov bot commented Jan 10, 2021

Codecov Report

Merging #38 (3672fd3) into master (408ebf5) will increase coverage by 15.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #38       +/-   ##
===========================================
+ Coverage   60.40%   75.41%   +15.01%     
===========================================
  Files          12       12               
  Lines        1139     1139               
===========================================
+ Hits          688      859      +171     
+ Misses        451      280      -171     
Impacted Files Coverage Δ
R/logger.R 91.17% <ø> (+67.22%) ⬆️
R/generate_template.R 100.00% <0.00%> (+2.32%) ⬆️
R/fw_helpers_external.R 95.23% <0.00%> (+2.38%) ⬆️
R/convert_template.R 100.00% <0.00%> (+5.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a9c930...3672fd3. Read the comment docs.

Copy link
Owner

@cb4ds cb4ds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested changes and approving since they are test-only changes. However note that Shiny 1.6 (RC branch) depends on development versions and non-cran packages so full testing will need to be done when Shiny 1.6 (and associated dependencies) make it onto CRAN.

@cb4ds cb4ds merged commit 2841d37 into cb4ds:master Jan 11, 2021
@cpsievert cpsievert deleted the shiny-1.6 branch January 11, 2021 23:14
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.

None yet

2 participants