503 shinytest2 for `landing_popup_module`

[m7pr]

Part of https://github.com/insightsengineering/coredev-tasks/issues/503 Just a warm-up. Trying to understand shinytest2 tests in action. Took landing_popup_module to the battlefield.

[m7pr]

Hey @averissimo @gogonzo @vedhav

Took a stab at shinytest2 tests. Any chance you can review the proposed tests for landing_popup_module and review comments? I think the last test does not execute the onclick/click.

[m7pr]

I think the whole onclick does not work when clicked for app

[github-actions[bot]]

CLA Assistant Lite bot ✅ All contributors have signed the CLA

[m7pr]

Thanks @averissimo for letting me know that app$get_text exists : D And also for showing the better selector statements

[github-actions[bot]]

Unit Tests Summary

  1 files   28 suites   11s :stopwatch: 234 tests 208 :white_check_mark: 26 :zzz: 0 :x: 445 runs  419 :white_check_mark: 26 :zzz: 0 :x:

Results for commit dbd96a63.

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

[github-actions[bot]]

Unit Test Performance Difference

Test Suite	$Status$	Time on main	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
shinytest2-landing_popup	👶		$+0.07$	$+5$	$+5$	$0$	$0$

Additional test case details

| Test Suite | $Status$ | Time on `main` | $±Time$ | Test Case | |:-----|:----:|:----:|:----:|:-----| | shinytest2-landing_popup | 👶 | | $+0.01$ | e2e_app_with_customized_landing_popup_module_creates_modal_containing_specified_title_content_and_buttons | | shinytest2-landing_popup | 👶 | | $+0.01$ | e2e_app_with_default_landing_popup_module_creates_modal_containing_a_button | | shinytest2-landing_popup | 👶 | | $+0.01$ | e2e_teal_app_with_landing_popup_module_initializes_with_no_errors | | shinytest2-landing_popup | 👶 | | $+0.01$ | e2e_when_customized_button_in_landing_popup_module_is_clicked_it_redirects_to_a_certain_page | | shinytest2-landing_popup | 👶 | | $+0.02$ | e2e_when_default_landing_popup_module_is_closed_it_shows_the_underlying_teal_app |

Results for commit 1c29a4c40708a4ad6ac1a32ec2c5fe76b9ceb698

♻️ This comment has been updated with latest results.

[m7pr]

Hey @averissimo maybe you have an idea why the buttons on

e2e: when customized button in landing_popup_module is clicked, it redirects to a certain page

test do not redirect after clicking. Trying to test the onclick functionality but ended up in a dead end.

[averissimo]

I tried a bunch of JS ideas and can't get the event to be triggered in shinytest2. This may be a limitation of {shinytest2}

• Running the onclick function from the property itself
• Running open directly (app$run_js("window.open('http://google.com', '_blank')"))
• ....

The best I can get is the function definition from the element itself

app$get_js("document.getElementById(\"read\").onclick.toString()")
#> [1] "function onclick(event) {\nwindow.open('http://google.com', '_blank')\n}"

[github-actions[bot]]

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                   234     234  0.00%    29-481
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                              1901     740  61.07%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  --------
R/TealAppDriver.R       +3      +3  +100.00%
TOTAL                   +3      +3  -0.10%

Results for commit: dbd96a6340207cfcffa58d687ec29d406a6cb188

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

[m7pr]

So @averissimo you say we can't click the button to achieve the onlick action? We can only extract the information that is included in the button and then compare if this contains what it should contain? This is also fine!

[averissimo]

I don't think the problem is with the onclick per se, but maybe the window JS API is restricted. Consider the example below:

On click works for the first one, but not the second.

library(shiny)

app <- shinyApp(
  fluidPage(
    div(
      id = "clicky",
      "click me",
      onclick = "Shiny.setInputValue('test', { 'key1' : 'val1', 'key2' : 'val2' }, {priority: 'event'})"
    ),
    div(
      id = "clicky2",
      "click me",
      onclick = "window.open('http://google.com', '_blank')"
    ),
    verbatimTextOutput("test")
  ),

  function(input, output, session) {
    output$test <- renderPrint({
      str(input$test)
    }) 
  }
)

driver <- shinytest2::AppDriver$new(app)

driver$view()
driver$click(selector = "#clicky")
driver$click(selector = "#clicky2")

[m7pr]

Thanks. But what matters the most that you can get the JS code from under the button info

app$get_js("document.getElementById(\"read\").onclick.toString()")

And this way we can actually see what was put during the app creation, and this is what I wanted to test. So I will update the test and we can call it a day!

[m7pr]

Per your last commit @averissimo , I think we now have a very concise way of doing this test with app$get_onclick("#read")

[m7pr]

Ah, I see you are referring to value which gets changed by JS when a button is clicked. Hmmm

[averissimo]

Yes, I'm not sure about it though as this is a Shiny functionality, not a landing module one.

Maybe we should only test that the landing popup appears. Everything else is Shiny-related. It's a slippery slope if we start to test Shiny and htmltools themselves.

[m7pr]

got it @averissimo and totally agree. I would stop the develpoment and keep the state as it's currently is.

[m7pr]

@averissimo would you be able to have one last final look in here when you have time? no pressure

[m7pr]

ugh, there is test failing for reporter part. but this PR does not involve testing reporter

[m7pr]

it was failing on the landing_popup test. some commits were missing. fixed the issue

[averissimo]

Other occurrences of getting attributes in the code could use this new method.

I can create a follow-up PR to this one and change those if you accept the changes :relaxed:

[vedhav]

Please add skip_if_too_deep(5) too when you get a chance.

[m7pr]

I can create a follow-up PR to this one and change those if you accept the changes

@averissimo sure! go for it, thanks!

[m7pr]

@averissimo thanks for the proposition of $get_attr. I think this will be super useful

[m7pr]

cleaned up some documentation and just pushed it for final review