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

Implement shinytest2 for tmc #1108

Closed
1 of 42 tasks
donyunardi opened this issue Mar 22, 2024 · 12 comments · Fixed by #1126, #1131, #1129, #1127 or #1125
Closed
1 of 42 tasks

Implement shinytest2 for tmc #1108

donyunardi opened this issue Mar 22, 2024 · 12 comments · Fixed by #1126, #1131, #1129, #1127 or #1125
Assignees
Labels

Comments

@donyunardi
Copy link
Contributor

donyunardi commented Mar 22, 2024

This is a continuation of #503

Using the shinytest2 helper class that we built for teal, let extend the feature for tmg.

We have 36 teal module functions in tmc

Note

  • We stick to one app per test and open issue to optimize this by exploring one app for all tests in a module.
  • Create function: app_driver_\<name of module\>
    • create this in tests/testthat/helper-TealAppDriver.R so we can call it as a function in the test case.
  • Test if the example apps for the modules tm_* can be run without shiny errors
    • We won't be doing a snapshot test or confirm if the exact output based on the encoding. We will only check if the visualization is generated when the app initialized.
    • Run the app using different arguments other than what's provided in the example (when applicable)
  • Test if visualization is updated when encoding is updated
  • Prefix all the external function (i.e. within and specifying the module, everywhere!)

Pre-requirements

Modules

Wave 1

Wave 2

Wave 3

Wave 4

Wave 5

Wave 6

Wave 7

Checklist of things to check on the feature branch

  • Rename get_active_module_tws_output and get_active_module_pws_output with get_active_module_table_output and get_active_module_plot_output respectively after teal #1210 is merged. Also look for rvest::html_table() and use get_active_module_table_output for DT tables.
  • Check if all the tests have skip_if_too_deep(5) and app_driver$stop()
  • Make sure the test description is standard: Begin with a Capital letter and end with . uses variables like arm_var instead of arm_var-variable or ARM variable
  • Verify that all the module params are covered in the test. Especially check for cases with fixed data transform. (leave parameter with deprecated status)
  • prefix teal.data:: for teal_data(), datanames() and join_keys where ever its missing.
@vedhav
Copy link
Contributor

vedhav commented Apr 24, 2024

There are two things that we are not going to focus during the scope of implementing the shinytest2 tests:

  1. These 10 modules tm_a_gee, tm_a_mmm, tm_g_forest_rsp, forest_tte, tm_g_km, tm_t_ancova, tm_t_binary_outcome, tm_t_coxreg, tm_t_logistic, and tm_t_tte uses a draggable widget which was not easy to interact with using the app driver.
Screenshot 2024-04-24 at 8 03 34 PM
  1. We have to circle back and check if we cover all the conditional widgets for the encoding that is being created.

@vedhav
Copy link
Contributor

vedhav commented Apr 25, 2024

Template for the common test patterns we use.

Common first test for all the modules

For plot modules

testthat::test_that("e2e - tm_xxxxx: Module initializes in teal without errors and produces plot output.", {
  skip_if_too_deep(5)
  app_driver <- app_driver_tm_xxxxx()
  app_driver$expect_no_shiny_error()
  app_driver$expect_no_validation_error()

  testthat::expect_match(
    app_driver$get_active_module_pws_output("myplot"),
    "data:image/png;base64,"
  )

  app_driver$stop()
})

For table modules

testthat::test_that("e2e - tm_xxxxx: Module initializes in teal without errors and produces table output.", {
  skip_if_too_deep(5)
  app_driver <- app_driver_tm_xxxxx()
  app_driver$expect_no_shiny_error()
  app_driver$expect_no_validation_error()

  testthat::expect_true(
    app_driver$is_visible(app_driver$active_module_element("table-table-with-settings"))
  )
  app_driver$stop()
})

Check if the the module initializes with proper defaults

testthat::test_that("e2e - tm_xxxxx: Starts with specified label, XXXXX, ...", {
  skip_if_too_deep(5)
  app_driver <- app_driver_tm_xxxxx()

  testthat::expect_equal(
    app_driver$get_text("#teal-main_ui-root-active_tab > li.active > a"),
    "Module tab label"
  )
  testthat::expect_equal(
    app_driver$get_active_module_input("XXXXX-dataset_ADXX_singleextract-select"),
    "XXXXX Value"
  )
  # ...

  app_driver$stop()
})

Check is a different selection works ✅

For a plot module

testthat::test_that("e2e - tm_xxxxx: Selecting XXXXX changes plot and doesn't throw validation errors.", {
  skip_if_too_deep(5)
  app_driver <- app_driver_tm_xxxxx()
  plot_before <- app_driver$get_active_module_pws_output("myplot")
  app_driver$set_active_module_input("XXXXX-dataset_ADXX_singleextract-select", "XXXXX New Value")
  testthat::expect_false(
    identical(
      plot_before,
      app_driver$get_active_module_pws_output("myplot")
    )
  )
  app_driver$expect_no_validation_error()
  app_driver$stop()
})

For a table module

testthat::test_that("e2e - tm_xxxxx: Selecting XXXXX changes the table and does not throw validation errors.", {
  skip_if_too_deep(5)
  app_driver <- app_driver_tm_xxxxx()
  table_before <- app_driver$get_active_module_tws_output("table")
  app_driver$set_active_module_input("XXXXX-dataset_ADXX_singleextract-select", "XXXXX New Value")
  testthat::expect_false(identical(table_before, app_driver$get_active_module_tws_output("table")))
  app_driver$expect_no_validation_error()
  app_driver$stop()
})

Check if no selection works ✅

For a plot module

testthat::test_that("e2e - tm_xxxxx: Deselecting XXXXX changes plot and doesn't throw validation error.", {
  skip_if_too_deep(5)
  app_driver <- app_driver_tm_xxxxx()
  plot_before <- app_driver$get_active_module_pws_output("myplot")
  app_driver$set_active_module_input("XXXXX-dataset_ADXX_singleextract-select", NULL)
  testthat::expect_false(identical(plot_before, app_driver$get_active_module_pws_output("myplot")))
  app_driver$expect_no_validation_error()
  app_driver$stop()
})

For a table module

testthat::test_that("e2e - tm_xxxxx: Deselecting XXXXX changes table and doesn't throw validation error.", {
  skip_if_too_deep(5)
  app_driver <- app_driver_tm_xxxxx()
  table_before <- app_driver$get_active_module_tws_output("table")
  app_driver$set_active_module_input("XXXXX-dataset_ADXX_singleextract-select", NULL)
  testthat::expect_false(identical(table_before, app_driver$get_active_module_tws_output("table")))
  app_driver$expect_no_validation_error()
  app_driver$stop()
})

Check if no selection shows validation error ❌

For a plot module

testthat::test_that("e2e - tm_xxxxx: Deselecting XXXXX column throws validation error.", {
  skip_if_too_deep(5)
  app_driver <- app_driver_tm_xxxxx()
  app_driver$set_active_module_input("XXXXX-dataset_ADXXX_singleextract-select", NULL)
  testthat::expect_identical(app_driver$get_active_module_pws_output("myplot"), character(0))
  testthat::expect_identical(
    app_driver$active_module_element_text("XXXXX-dataset_ADXXX_singleextract-select_input > div > span"),
    "Validation message"
  )
  app_driver$expect_validation_error()
  app_driver$stop()
})

For a table module

testthat::test_that("e2e - tm_xxxxx: Deselection of XXXXX throws validation error.", {
  skip_if_too_deep(5)
  app_driver <- app_driver_tm_xxxxx()
  app_driver$set_active_module_input("XXXXX-dataset_ADXX_singleextract-select", NULL)
  testthat::expect_identical(app_driver$get_active_module_tws_output("table"), data.frame())
  app_driver$expect_validation_error()
  testthat::expect_equal(
    app_driver$active_module_element_text("XXXXX-dataset_ADXX_singleextract-select_input .shiny-validation-message"),
    "Validation message"
  )
  app_driver$stop()
})

averissimo added a commit that referenced this issue Apr 25, 2024
# Pull Request

Part of #1108 

### What is being tested
- Default values that can be changed
- Selecting `x` variable
- Deselecting `x` variable should give error
- Deselecting others does not throw error
- Selecting same variables as `x` has a validation error

---------

Signed-off-by: André Veríssimo <[email protected]>
Co-authored-by: kartikeya kirar <[email protected]>
m7pr added a commit that referenced this issue Apr 25, 2024
Part of #1108 

We can move `active_module_tws_output` to be a method of
`teal:::TealAppDriver`

---------

Signed-off-by: Marcin <[email protected]>
Co-authored-by: kartikeya kirar <[email protected]>
@gogonzo gogonzo reopened this Apr 26, 2024
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Signed-off-by: Vedha Viyash <[email protected]>
Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
vedhav added a commit that referenced this issue May 7, 2024
vedhav added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Signed-off-by: Vedha Viyash <[email protected]>
Co-authored-by: Marcin <[email protected]>
vedhav added a commit that referenced this issue May 7, 2024
Part of
#1108

---------

Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: André Veríssimo <[email protected]>
Co-authored-by: Vedha Viyash <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
kartikeyakirar added a commit that referenced this issue May 7, 2024
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
vedhav added a commit that referenced this issue May 7, 2024
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Signed-off-by: Vedha Viyash <[email protected]>
Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
kartikeyakirar added a commit that referenced this issue May 7, 2024
Part of #1108

---------

Co-authored-by: unknown <[email protected]>
vedhav added a commit that referenced this issue May 7, 2024
linksto  #1108

---------

Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: vedhav <[email protected]>
Co-authored-by: kartikeya kirar <[email protected]>
Co-authored-by: Vedha Viyash <[email protected]>
vedhav added a commit that referenced this issue May 7, 2024
# Pull Request

Part of #1108

Module has very specific set of variables that don't allow for many
choices

Fields not 100% tested:

- `cmdecod`: selected variable needs to be named CMDECOD due to
limitation in module's logic
- `cmtrt`: Could not manipulate data for this field to have any change
on output.
  - It seems that it is only used when CMDECOD has long values

---------

Co-authored-by: Vedha Viyash <[email protected]>
Co-authored-by: vedhav <[email protected]>
vedhav added a commit that referenced this issue May 9, 2024
Part of #1108

---------

Signed-off-by: Vedha Viyash <[email protected]>
Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: unknown <[email protected]>
vedhav added a commit that referenced this issue May 10, 2024
Closes #1108 

A unified branch for all partial PRs that will solve #1108 

Merge at the end. Place where we can unify PRs

---------

Signed-off-by: André Veríssimo <[email protected]>
Signed-off-by: Marcin <[email protected]>
Signed-off-by: Vedha Viyash <[email protected]>
Signed-off-by: kartikeya kirar <[email protected]>
Co-authored-by: m7pr <[email protected]>
Co-authored-by: André Veríssimo <[email protected]>
Co-authored-by: Vedha Viyash <[email protected]>
Co-authored-by: vedhav <[email protected]>
Co-authored-by: kartikeya kirar <[email protected]>
Co-authored-by: Dawid Kałędkowski <[email protected]>
Co-authored-by: Dawid Kałędkowski <[email protected]>
Co-authored-by: gogonzo <[email protected]>
Co-authored-by: vedhav <[email protected]>
Co-authored-by: kartikeyakirar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment