Skip to content

Conversation

@vedhav
Copy link
Contributor

@vedhav vedhav commented Oct 10, 2025

No description provided.

@gogonzo
Copy link
Contributor

gogonzo commented Oct 10, 2025

@vedha I don't know whether it is needed, maybe we can remove constructor entirely and do this in tests:

app_dr <- shinytest2::AppDriver$new( # or TealAppDriver
  init(
    data = teal_data() |> within(iris <- iris),
    modules = modules(example_module())
  ) |>
    do.call(what = shiny::shinyApp)
)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 10, 2025

badge

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  --------------------------------------------------------------------------------------------------------------------------------------
R/after.R                            59      21  64.41%   42-52, 64, 69, 77-79, 81-89, 100, 104-105
R/checkmate.R                        24       0  100.00%
R/dummy_functions.R                  61       2  96.72%   44, 46
R/include_css_js.R                   11       0  100.00%
R/init.R                            152       1  99.34%   299
R/landing_popup_module.R             34      10  70.59%   44-53
R/module_bookmark_manager.R         153     117  23.53%   54-58, 78-133, 138-139, 151, 198, 233-310
R/module_data_summary.R             177       8  95.48%   40, 50, 205, 236-240
R/module_filter_data.R               64       0  100.00%
R/module_filter_manager.R           229      50  78.17%   72-81, 89-94, 107-111, 116-117, 290-313, 339, 366, 378, 385-386
R/module_init_data.R                 84       6  92.86%   38-43
R/module_nested_tabs.R              371      37  90.03%   163, 267-282, 302-306, 324, 361, 479-482, 486-489, 493-496, 541
R/module_session_info.R              18       0  100.00%
R/module_snapshot_manager.R         271     194  28.41%   103-112, 120-144, 163-164, 181-210, 214-229, 231-238, 245-275, 279, 283-287, 289-295, 298-311, 314-322, 352-366, 369-380, 383-397, 410
R/module_source_code.R               69       0  100.00%
R/module_teal_data.R                149      76  48.99%   43-149
R/module_teal_lockfile.R            131      53  59.54%   45-57, 60-62, 76, 86-88, 100-102, 110-119, 122, 124, 126-127, 142-146, 161-162, 177-186
R/module_teal_reporter.R            122       9  92.62%   60, 77-78, 81, 98, 128, 142, 144, 158
R/module_teal_with_splash.R          33      33  0.00%    24-61
R/module_teal.R                     213      28  86.85%   130, 134-135, 145-146, 186, 204-220, 222, 255-256, 263-264
R/module_transform_data.R           116       6  94.83%   46, 130-134
R/modules.R                         291      51  82.47%   170-174, 229-232, 356-376, 384, 390, 567-573, 586-594, 609-624
R/reporter_previewer_module.R        41      41  0.00%    22-85
R/show_rcode_modal.R                 31      31  0.00%    17-49
R/tdata.R                            14      14  0.00%    19-61
R/teal_data_module-eval_code.R       23       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                 20       0  100.00%
R/teal_data_utils.R                  49       0  100.00%
R/teal_modifiers.R                   57       0  100.00%
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      63       0  100.00%
R/teal_transform_module.R            45       0  100.00%
R/TealAppDriver.R                   305     305  0.00%    51-630
R/utils.R                           291      48  83.51%   402-451, 539-548
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   114-392
R/zzz.R                              19      15  21.05%   4-22
TOTAL                              3916    1193  69.54%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  --------
R/TealAppDriver.R       +8      +8  +100.00%
TOTAL                   +8      +8  -0.14%

Results for commit: 8cde224

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Oct 10, 2025

Unit Tests Summary

  1 files   29 suites   2m 34s ⏱️
331 tests 265 ✅ 6 💤 0 ❌ 60 🔥
533 runs  467 ✅ 6 💤 0 ❌ 60 🔥

For more details on these errors, see this check.

Results for commit 8cde224.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 10, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
module_teal 💔 $119.22$ $+3.32$ $0$ $0$ $0$ $0$

Results for commit ff8e9f9

♻️ This comment has been updated with latest results.

@gogonzo
Copy link
Contributor

gogonzo commented Oct 10, 2025

I also think about active_ns - I think we can have a better way to handle this. There is more and more coming to "module container". We have now:

base:
  base-wrapper
    - base-reporter_add_container
    - base-src_container
    - base-filter_panel
    - base-data_summary
    - base-module

The list is unlimited. Meanwhile we have:

  • active_ns() - returns a list of ids filter_panel, module, data_summary
  • active_module_ns() - returns character id of module
  • active_data_summary_ns() - returns character id of data_summary
  • active_filters_ns() - returns character id of filter_panel (comment: names mismatch)
  • active_module_element(target) - returns character id with #module_ns-target
    ... and many more

I think we can limit this to two functions, like:

  • active_id() - returns a list of id of all known teal elements (module, wrapper, filter_panel, data_summary, transformators, show_r_code, ...)
  • active_ns(prefix) returns ns of each known element, so that we can active_ns()$module("id_in_module")

reason is that TealAppDriver has too many methods which are created just to serve particular part of the application. We don't need to create a method to get an id or #selector of any element in the module-context. Let's make a "factory" method and we will save a lot of code.

@gogonzo
Copy link
Contributor

gogonzo commented Oct 15, 2025

@vedhav to close this one?

@m7pr
Copy link
Contributor

m7pr commented Oct 15, 2025

still 54 fails in R CMD CHECK

  1. └─TealAppDriver$new(...) at test-shinytest2-teal_modifiers.R:28:5
   2.   └─teal (local) initialize(...)
   3.     └─checkmate::assert_class(app$ui, "shiny.tag")
   4.       └─checkmate::makeAssertion(x, res, .var.name, add)
   5.         └─checkmate:::mstop(...)
  ── Error ('test-shinytest2-teal_modifiers.R:43:3'): e2e: modify_header displays custom header in the app ──
  Error in `initialize(...)`: Assertion on 'app$ui' failed: Must inherit from class 'shiny.tag', but has class 'function'.
  Backtrace:

https://github.com/insightsengineering/teal/actions/runs/18407812434/job/52451882436?pr=1629#step:44:583

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.

4 participants