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

712 - {shinytest2} for tm_g_bivariate #720

Merged
merged 39 commits into from
Apr 19, 2024
Merged

Conversation

vedhav
Copy link
Contributor

@vedhav vedhav commented Apr 11, 2024

Part of #712

@m7pr
Copy link
Contributor

m7pr commented Apr 11, 2024

Hey @vedhav I left you a few comments + requested your review on #718 so that maybe you can see what other things can be tested besides just using app$get_active_module_input()

@averissimo averissimo mentioned this pull request Apr 12, 2024
19 tasks
@m7pr
Copy link
Contributor

m7pr commented Apr 12, 2024

@vedhav I think there is a lot of ggplot:: prefixing that is missing in R/tm_g_bivariate.R - I do experience issues when testing R/tm_g_association.R

@m7pr
Copy link
Contributor

m7pr commented Apr 12, 2024

It was only one : 58686ee

@vedhav
Copy link
Contributor Author

vedhav commented Apr 12, 2024

I added the prefixes now. I think you committed the changes in your branch by mistake @m7pr

@vedhav vedhav marked this pull request as ready for review April 12, 2024 13:10
Copy link
Contributor

github-actions bot commented Apr 12, 2024

Unit Tests Summary

  1 files   16 suites   6m 56s ⏱️
 80 tests  80 ✅ 0 💤 0 ❌
316 runs  316 ✅ 0 💤 0 ❌

Results for commit 90ab7ed.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Apr 12, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-tm_g_bivariate 👶 $+43.12$ $+44$ $0$ $0$ $0$
shinytest2-tm_g_scatterplot 💔 $73.87$ $+25.02$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
shinytest2-tm_g_bivariate 👶 $+6.20$ e2e_tm_g_bivariate_coloring_options_are_hidden_when_coloring_is_toggled_off
shinytest2-tm_g_bivariate 👶 $+6.18$ e2e_tm_g_bivariate_facetting_options_are_hidden_when_facet_is_toggled_off
shinytest2-tm_g_bivariate 👶 $+6.11$ e2e_tm_g_bivariate_module_is_initialised_with_the_specified_defaults
shinytest2-tm_g_bivariate 👶 $+24.64$ e2e_tm_g_bivariate_setting_encoding_inputs_produces_outputs_without_validation_errors
shinytest2-tm_g_scatterplot 💔 $7.10$ $+25.09$ e2e_tm_g_scatterplot_module_is_initialised_with_the_specified_defaults

Results for commit 235d88d

♻️ This comment has been updated with latest results.

@m7pr m7pr added the core label Apr 16, 2024
@m7pr
Copy link
Contributor

m7pr commented Apr 18, 2024

@averissimo besides this comment https://github.com/insightsengineering/teal.modules.general/pull/720/files#r1565844323
I think we are ready to go. Would you be able to update this PR?

@m7pr
Copy link
Contributor

m7pr commented Apr 18, 2024

I actually did that here 8a5b7cb

@m7pr
Copy link
Contributor

m7pr commented Apr 18, 2024

Do we need tests for color, fill and size parameters?
image

Copy link
Contributor

@m7pr m7pr left a comment

Choose a reason for hiding this comment

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

Hey, this PR was so abandoned that I finished it with my own suggestions : P

@averissimo averissimo enabled auto-merge (squash) April 19, 2024 12:06
@averissimo averissimo merged commit cbdc1ca into main Apr 19, 2024
24 checks passed
@averissimo averissimo deleted the 712-tm_g_bivariate@main branch April 19, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants