Skip to content

replace substitute-based generator with plain closure#69

Merged
donyunardi merged 8 commits intomainfrom
update_check_pick_generator@main
May 7, 2026
Merged

replace substitute-based generator with plain closure#69
donyunardi merged 8 commits intomainfrom
update_check_pick_generator@main

Conversation

@donyunardi
Copy link
Copy Markdown
Contributor

@donyunardi donyunardi commented May 6, 2026

Part of fix for https://github.com/insightsengineering/coredev-tasks/issues/750

I updated the .check_pick_generator to use plain closure rather than using rlang::new_function + substitute(). Based on my research, rlang::new_function() + substitute() hides the generated function body from covr, resulting in 0% coverage for helpers.R, even though we already have unit tests for is_pick_multiple, is_pick_fixed, and is_pick_ordered.

The plain closure return the exact same thing with previous solution.

It only returns 37.5% but I guess this is covr limitation when using factory pattern solution.

image image

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

badge

Code Coverage Summary

Filename                  Stmts    Miss  Cover    Missing
----------------------  -------  ------  -------  -------------------------------------------------------------
R/as_picks.R                140      21  85.00%   63, 167-171, 182-186, 201-207, 229-231
R/assertion.R                 5       0  100.00%
R/call_utils.R              147      33  77.55%   23-28, 65, 132-138, 259, 278-289, 292, 317-325
R/helpers.R                   9       0  100.00%
R/interaction.R              42      26  38.10%   39-97
R/module_merge.R            266      14  94.74%   303, 310-320, 337, 616
R/module_picks.R            318      23  92.77%   47-53, 71, 109-110, 298-300, 302-306, 429, 478, 516, 528, 536
R/picks.R                   183       1  99.45%   335
R/print.R                    36       2  94.44%   50, 58
R/resolver.R                141      19  86.52%   80-83, 110-118, 284-289
R/tidyselect-helpers.R       29       0  100.00%
R/tm_merge.R                 54      54  0.00%    42-102
R/ui_containers.R            42       0  100.00%
R/zzz.R                       5       5  0.00%    3-11
TOTAL                      1417     198  86.03%

Diff against main

Filename            Stmts    Miss  Cover
----------------  -------  ------  --------
R/call_utils.R          0     +11  -7.48%
R/helpers.R            -3     -12  +100.00%
R/interaction.R         0     +25  -59.52%
R/module_merge.R       +9     +12  -4.48%
R/resolver.R            0      +4  -2.84%
TOTAL                  +6     +40  -2.78%

Results for commit: ee46d0e

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Unit Tests Summary

  1 files   10 suites   17s ⏱️
255 tests 241 ✅ 14 💤 0 ❌
383 runs  368 ✅ 15 💤 0 ❌

Results for commit ee46d0e.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
interaction 💀 $0.22$ $-0.22$ interaction_vars_filters_table_when_specific_values_are_subset

Results for commit 9acc38f

♻️ This comment has been updated with latest results.

…sengineering/teal.picks into update_check_pick_generator@main
@donyunardi
Copy link
Copy Markdown
Contributor Author

Roxygen Man Pages Auto Update was triggered, updating/adding the .Rd files.

@donyunardi donyunardi added the core label May 6, 2026
@averissimo averissimo self-assigned this May 6, 2026
@osenan
Copy link
Copy Markdown
Contributor

osenan commented May 6, 2026

I have added a change in merge_srv in order to have coverage for all function files. The module function had a big coverage, but `covr::package_coverage was not picking it.

Comment thread R/helpers.R Outdated
@donyunardi donyunardi requested a review from averissimo May 6, 2026 22:12
Copy link
Copy Markdown
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.

💯

@donyunardi donyunardi merged commit d735f14 into main May 7, 2026
27 of 28 checks passed
@donyunardi donyunardi deleted the update_check_pick_generator@main branch May 7, 2026 08:17
@github-actions github-actions Bot locked and limited conversation to collaborators May 7, 2026
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.

3 participants