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]

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]

Unit Tests Summary

  1 files   28 suites   11s ⏱️
234 tests 208 ✅ 26 💤 0 ❌
445 runs  419 ✅ 26 💤 0 ❌

Results for commit dbd96a6.

♻️ This comment has been updated with latest results.

[github-actions]

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 1c29a4c

♻️ 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]

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: dbd96a6

Minimum allowed coverage is 80%

♻️ 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!

[averissimo]

Small changes:

• I think we can avoid the :nth-child(1) as only 1 element will have data-dismiss attribute.
• Please unify how buttons are specified on all PRs with button or .btn or .btn-default
  • I would prefer one of the first 2, but I think any of them would be fine

---

It would be nice to detect onclick behavior via without resorting to string matching.
I've come up with this: (after failing to use a r shiny::textInput with js Shiny::setInputValue call to change it)

onclick_text <- "document.getElementById('unique_input_id').value = 'Lorem ipsum';"

  app <- TealAppDriver$new(
    data = simple_teal_data(),
    modules = modules(
      landing_popup_module(
        buttons = actionButton("read", "Read more", onclick = onclick_text),
        content = tags$input(id = "unique_input_id")
      ),
      example_module()
    )
  )
  app$wait_for_idle(timeout = default_idle_timeout)

  onclick_text_app <- app$click(selector = "#read")

  # Looks for side-effects of button click (i.e. input value change).
  testthat::expect_equal(
    app$get_js("document.getElementById('unique_input_id').value"),
    "Lorem ipsum"
  )

[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]

Thanks for the changes, I have one more comment, let's discuss it 😄

It boils down to the specificity of get_onclick.

It would be nice to:

• have access to other attributes in the future tests
• getting onclick from <a> elements or another element
  • (<div> on screenshot below also has onclick attr)

As extra bonus, it also becomes a vectorial function.

app$get_attr("button", "onclick")
#> [1] "toggleFilterPanel();"                       NA                                          
#> [3] NA                                           NA                                          
#> [5] NA                                           NA                                          
#> [7] "window.open('http://google.com', '_blank')" "window.close()"                            

[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 ☺️

[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

[averissimo]

Looks good! Nice work 💯