insightsengineering / teal

Exploratory Web Apps for Analyzing Clinical Trial Data
https://insightsengineering.github.io/teal/
Other
168 stars 33 forks source link

1163 extend `TealAppDriver$new()` and `TealAppDriver$click()` with `$wait_for_idle()` calls #1171

Closed m7pr closed 3 months ago

m7pr commented 4 months ago

Close #1163

Questions:

averissimo commented 4 months ago
  • Should we add timeout parameter to initialize that will be passed to self$wait_for_idle() or add ... in self$wait_for_idle(...) at the end of the call. So that during TealAppDriver$new() you can pass timeout =.

  • app$navigate_teal_tab uses private$idle_timeout (self$wait_for_idle(timeout = private$idle_timeout)). Should we use private$idle_timeout in TealAppDrvier$new where we call $wait_for_idle(). Also, should we set private$idle_timeout in TealAppDrvier$new

I think we should change the default value of the private element private$timeout to 20s instead of this non-default field that's already on main and reimplementing wait_for_idle.

@vedhav was there a specific rationale to create private$idle_timeout instead of changing the default value of private$timeout?

github-actions[bot] commented 4 months ago

badge

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------
R/dummy_functions.R                  30      21  30.00%   21-33, 36-43
R/get_rcode_utils.R                  31       1  96.77%   50
R/include_css_js.R                   22       0  100.00%
R/init.R                             86      31  63.95%   108-115, 161-162, 164, 176-197, 227-228, 230
R/landing_popup_module.R             25      25  0.00%    61-87
R/module_filter_manager.R           107      36  66.36%   37-43, 50-58, 67-72, 195, 200-213
R/module_nested_tabs.R              154      58  62.34%   39-112, 128, 180, 202, 228
R/module_snapshot_manager.R         209     157  24.88%   87-99, 127-136, 140-152, 154-161, 168-182, 186-188, 190-195, 198-208, 211-227, 236-251, 265-288, 291-302, 305-311, 325, 343-366
R/module_tabs_with_filters.R         76      33  56.58%   33-68, 100, 116
R/module_teal_with_splash.R         114       4  96.49%   110, 131, 197-198
R/module_teal.R                     106      29  72.64%   57, 68, 77, 150-151, 157, 176-207
R/modules.R                         152      26  82.89%   127-130, 147-151, 206-209, 291-292, 344, 356-364, 418-421
R/reporter_previewer_module.R        18       2  88.89%   30, 34
R/show_rcode_modal.R                 19      19  0.00%    17-36
R/tdata.R                            53       1  98.11%   154
R/teal_data_module-eval_code.R       27       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                  6       0  100.00%
R/teal_reporter.R                    62       5  91.94%   69, 118-119, 122, 139
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      59      12  79.66%   137-150
R/TealAppDriver.R                   249     249  0.00%    43-531
R/utils.R                           173       1  99.42%   255
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   110-377
R/zzz.R                              12       8  33.33%   3-15
TOTAL                              1916     755  60.59%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  --------
R/TealAppDriver.R       +5      +5  +100.00%
TOTAL                   +5      +5  -0.16%

Results for commit: 13dc24e45e30ee038baeb095f41921d47d13cc95

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

github-actions[bot] commented 4 months ago

Unit Tests Summary

  1 files   28 suites   2m 21s :stopwatch: 234 tests 234 :white_check_mark: 0 :zzz: 0 :x: 497 runs  497 :white_check_mark: 0 :zzz: 0 :x:

Results for commit 13dc24e4.

:recycle: This comment has been updated with latest results.

averissimo commented 4 months ago

Ps. I know it was me that suggested the reimplemented wait_for_idle, but changing the default private field might be enough to achieve the same purpose 😅

m7pr commented 4 months ago

@averissimo no worries, but the part of adding $wait_for_idle() calls to the $new() and $click() methods is still valid : )

averissimo commented 4 months ago

@averissimo no worries, but the part of adding $wait_for_idle() calls to the $new() and $click() methods is still valid : )

For sure! This will help us avoid a bunch of statements on tests :+1:

github-actions[bot] commented 3 months ago

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-filter_panel 💚 $19.86$ $-2.10$ $0$ $0$ $0$ $0$
shinytest2-reporter 💔 $14.64$ $+18.33$ $0$ $0$ $0$ $0$
shinytest2-teal_data_module 💔 $10.37$ $+1.07$ $0$ $0$ $0$ $0$
Additional test case details | Test Suite | $Status$ | Time on `main` | $±Time$ | Test Case | |:-----|:----:|:----:|:----:|:-----| | shinytest2-filter_panel | 💚 | $7.18$ | $-1.12$ | e2e_filtering_a_module_specific_filter_is_not_refected_in_other_unshared_modules | | shinytest2-filter_panel | 💚 | $7.10$ | $-1.09$ | e2e_filtering_a_module_specific_filter_is_refected_in_other_shared_module | | shinytest2-reporter | 💔 | $8.94$ | $+17.14$ | e2e_adding_a_report_card_in_a_module_adds_it_in_the_report_previewer_tab |

Results for commit 5441cd12c4a98f9983ab898dada672b6c2153a19

♻️ This comment has been updated with latest results.

m7pr commented 3 months ago

Thanks guys for taking care of this!