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

Stop R process if AppDriver fails in test-examples #820

Merged
merged 3 commits into from
Dec 17, 2024

Conversation

averissimo
Copy link
Contributor

Pull Request

Fixes insightsengineering/teal.modules.clinical#1302 on teal.modules.general

@averissimo averissimo changed the title feat: stop R process after AppDriver fails Stop R process after AppDriver fails in test-examples Dec 16, 2024
Copy link
Contributor

github-actions bot commented Dec 16, 2024

badge

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  -------------------------------------------------
R/tm_a_pca.R                    884     884  0.00%    138-1155
R/tm_a_regression.R             772     772  0.00%    162-1036
R/tm_data_table.R               215     215  0.00%    110-380
R/tm_file_viewer.R              173     173  0.00%    47-255
R/tm_front_page.R               133     122  8.27%    73-231
R/tm_g_association.R            340     340  0.00%    143-556
R/tm_g_bivariate.R              685     421  38.54%   315-794, 835, 946, 963, 981, 992-1014
R/tm_g_distribution.R          1110    1110  0.00%    155-1411
R/tm_g_response.R               364     364  0.00%    161-601
R/tm_g_scatterplot.R            736     736  0.00%    244-1085
R/tm_g_scatterplotmatrix.R      296     277  6.42%    182-516, 577, 591
R/tm_missing_data.R            1111    1111  0.00%    122-1408
R/tm_outliers.R                1031    1031  0.00%    162-1342
R/tm_t_crosstable.R             261     261  0.00%    148-459
R/tm_variable_browser.R         830     825  0.60%    89-1081, 1119-1303
R/utils.R                       156     125  19.87%   81-266, 296-327, 348, 351, 356, 371-392, 403, 408
R/zzz.R                           2       2  0.00%    2-3
TOTAL                          9099    8769  3.63%

Diff against main

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

Results for commit: 4e9a0b2

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Dec 16, 2024

Unit Tests Summary

  1 files   22 suites   12m 47s ⏱️
145 tests 107 ✅ 38 💤 0 ❌
476 runs  438 ✅ 38 💤 0 ❌

Results for commit 4e9a0b2.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Dec 16, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
examples 💚 $6.81$ $-6.25$ $-1$ $0$ $0$ $0$
shinytest2-tm_a_pca 💚 $119.99$ $-4.46$ $0$ $0$ $0$ $0$
shinytest2-tm_a_regression 💚 $52.68$ $-2.08$ $0$ $0$ $0$ $0$
shinytest2-tm_file_viewer 💚 $29.97$ $-1.48$ $0$ $0$ $0$ $0$
shinytest2-tm_g_association 💚 $31.40$ $-1.50$ $0$ $0$ $0$ $0$
shinytest2-tm_g_bivariate 💚 $74.60$ $-1.75$ $0$ $0$ $0$ $0$
shinytest2-tm_g_scatterplot 💚 $74.42$ $-1.48$ $0$ $0$ $0$ $0$
shinytest2-tm_outliers 💚 $108.73$ $-2.14$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
examples 💚 $1.71$ $-1.63$ example_add_facet_labels.Rd
examples 💚 $1.08$ $-1.07$ example_tm_g_distribution.Rd

Results for commit 1fd2b00

♻️ This comment has been updated with latest results.

@averissimo averissimo changed the title Stop R process after AppDriver fails in test-examples Stop R process if AppDriver fails in test-examples Dec 16, 2024
Copy link
Contributor

@llrs-roche llrs-roche left a comment

Choose a reason for hiding this comment

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

Change looks good, I tested ran the test-examples.R file and all test report: "Skip: example-...Rd\nReason: testing depth 3 is below current testing specification 5"

Is this something that could affect end-users? Is it something that could be improved for them?

@averissimo
Copy link
Contributor Author

averissimo commented Dec 17, 2024

It doesn't affect any users as all end-to-end/interface testing is behind this "testing depth"

It's a testing specific bug, but if it appears it will possibly crash the system.

To test out the scenario this PR is fixing you can install teal before the decorator branch is merged and run the tests.

This should create a bunch of failed tests and you can check on the process list if there are no lingering R processes

renv::install("insightsengineering/teal@161e48056964623c2780c96b13b71c704dcadb1a")
withr::local_envvar("TESTING_DEPTH" = 5)
devtools::test(filter = "examples")

@averissimo averissimo merged commit 5c37681 into main Dec 17, 2024
26 checks passed
@averissimo averissimo deleted the test_examples@main branch December 17, 2024 11:52
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: App driver R process is not stopped during fails of test-examples.R
2 participants