Skip to content

Conversation

gogonzo
Copy link
Contributor

@gogonzo gogonzo commented Aug 15, 2025

Install teal.slice as important fix has been merged recently (insightsengineering/teal.slice#663). Please remember to install teal.slice locally if you want to test this branch - shinytest2 is run in the separate session so every packages must be installed (load_all has no effect).

In this PR:

  • needed to replace hardcoded ids of filter panel inputs with a functional approach !!ns_fs(<input path>) := <input value> . Thanks to this we just need to change "id" in one place if we ever change the namespace in teal or teal.slice

Copy link
Contributor

github-actions bot commented Aug 15, 2025

Unit Tests Summary

 1 files  15 suites   9s ⏱️
56 tests 43 ✅ 13 💤 0 ❌
86 runs  73 ✅ 13 💤 0 ❌

Results for commit ff62b01.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Aug 15, 2025

badge

Code Coverage Summary

Filename                Stmts    Miss  Cover    Missing
--------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------
R/adtteSpec.R             162     116  28.40%   253-391
R/assaySpec.R              49      36  26.53%   103-142
R/checkmate.R              18       9  50.00%   110-118
R/experimentSpec.R         91      58  36.26%   97, 215-284
R/geneSpec.R              257     154  40.08%   154-169, 296-481
R/sampleVarSpec.R         229     103  55.02%   293, 312-321, 327-334, 336, 344-356, 358-359, 361, 364, 372-376, 378-392, 397-421, 424-428, 430, 437-447, 449-450, 458, 503-520
R/tm_g_barplot.R          193     156  19.17%   41-69, 133-287
R/tm_g_boxplot.R          208     163  21.63%   42-70, 142-299
R/tm_g_forest_tte.R       213     184  13.62%   60-95, 156-328
R/tm_g_km.R               207     172  16.91%   63-95, 160-327
R/tm_g_pca.R              384     288  25.00%   36-59, 181-496
R/tm_g_quality.R          323     251  22.29%   18-113, 210-459
R/tm_g_scatterplot.R      186     152  18.28%   41-68, 131-280
R/tm_g_volcanoplot.R      212     178  16.04%   36-60, 120-303
R/utils.R                   6       0  100.00%
R/zzz.R                     2       2  0.00%    2-3
TOTAL                    2740    2022  26.20%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: ff62b01

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@gogonzo gogonzo marked this pull request as draft August 15, 2025 11:55
@gogonzo gogonzo changed the title Fix deep tests (WIP) Fix deep tests Aug 18, 2025
@gogonzo gogonzo added the core label Aug 18, 2025
- update snapshot
@gogonzo gogonzo marked this pull request as ready for review August 18, 2025 09:00
@gogonzo gogonzo changed the title (WIP) Fix deep tests Fix deep tests Aug 18, 2025
@averissimo averissimo self-assigned this Aug 19, 2025
Copy link
Contributor

@averissimo averissimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome improvement on the helper.

It works and can be merged as is, but I found 2 small problems that don't hold off the merge, but it would be nice to address (especially the 1.)

  1. A set_inputs that has no effect
  2. Too many # nolintr start comments (see #431)
    • This can be reviewed later on

@@ -78,7 +79,7 @@ test_that("scatterplot module works as expected in the test app", {
expect_identical(res, "GeneID:8086")

# Remove sample filter
app$click("teal-teal_modules-scatterplot-filter_panel-filters-MAE-subjects-MAE_SEX-remove")
app$click(ns_fp("MAE-subjects-MAE_SEX-remove"))
app$wait_for_idle()

res <- app$get_value(input = ns("x_spec-genes"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly related to this change, but in line 91

app$set_inputs(!!ns("experiment-name") := "hd2") is redundant and has no effect as it is already set before in line 42

This appears on the error

image

I recommend to remove it or create a way to skip if already "hd2"

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.

2 participants