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

"205-dynamic-tabs" and "213-tab-panels": shinytest2 tests failing due to click and tab selection discrepancy #130

Closed
MadhulikaTanuboddi opened this issue Aug 29, 2022 · 6 comments

Comments

@MadhulikaTanuboddi
Copy link
Contributor

MadhulikaTanuboddi commented Aug 29, 2022

These two app's shinytest2 tests have been failing since August 4, 2022 (almost on all OS and R version runs)

205-dynamic-tabs - This app runs with two bootswatch themes - cosmo3 and cosmo5. Tests are failing on cosmo5 only

213-tab-panels - Runs with default theme.

Upon my initial review of the snapshot differences, it looks like the snapshots are taken even before the click and tab selection actions are completed. For ex: notice the below snapshots (old and new).

I am not sure yet why these tests started failing since the above mentioned date and need to see what changes from other packages might have caused this.

In the meantime, I will be adding my findings here.

Screenshare.-.2022-08-29.9_59_37.AM.mp4

Screen Shot 2022-08-29 at 10 36 31 AM

Screen Shot 2022-08-29 at 10 36 49 AM

@MadhulikaTanuboddi
Copy link
Contributor Author

Could this be due to rstudio/bslib@22ac960?

@wch
Copy link
Collaborator

wch commented Aug 30, 2022

It sounds likely that this is related to the upgrade to Bootstrap 5.2.0.
For reference, here's the changelog for 5.2.0 (I don't know which item would have caused the changed results):
https://github.com/twbs/bootstrap/releases/tag/v5.2.0

@MadhulikaTanuboddi
Copy link
Contributor Author

MadhulikaTanuboddi commented Aug 30, 2022

Thank you. Agree. That PR was merged on August 3 and our tests started failing next day.

Screen Shot 2022-08-30 at 10 58 22 AM

cc: @cpsievert

@cpsievert
Copy link
Contributor

I'm pretty sure this is a result of this regression in Bootstrap 5.2 twbs/bootstrap#36947

Let's wait until I get a response from the maintainer in that issue to decide on what should be done next

@cpsievert
Copy link
Contributor

Update: they've merged my proposed fix for this issue in Bootstrap twbs/bootstrap#37151

We'll need to update bslib to the latest Bootstrap to see if that fix will fix these issues, but I'd prefer to wait for another official Bootstrap release before doing that

MadhulikaTanuboddi added a commit that referenced this issue Sep 23, 2022
Few notable differences and changes to mention

1. 010-download app - downloaded csv files now show the relevant name of the downloaded file
2. jQuery version update from 1.12.4 to 3.6.0
3. For 016-knitr-pdf and 112-generate-report app, new html report files generated
4. Cosmetic tab borderline changes to 30x apps
5. Most of the differences in this review are with the apps including DT tables. "dt-core" dependency upgrade from 1.11.3 to 1.12.1
6. Not accepting the new snaps of 205 and 213 apps. Waiting on a closure from #130
@MadhulikaTanuboddi
Copy link
Contributor Author

Bootstrap version upgraded on bslib - rstudio/bslib@0906a9f

A quick screenshare of current test results

Screenshare.-.2022-10-25.9_15_01.AM.mp4

Screen Shot 2022-10-25 at 9 16 36 AM

Carson mentioned that the snap differences are ok. " The dropdown appearing I think is a timing issue that we should fix, but it’s not the point of the test (and we shouldn’t delay the shiny release over it)"

cc: @schloerke

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

No branches or pull requests

3 participants