Fix recursive page numbering bug

edelarua commented 2 months ago

Closes #286

github-actions[bot] commented 2 months ago

Unit Tests Summary

1 files 8 suites 13s :stopwatch: 48 tests 48 :white_check_mark: 0 :zzz: 0 :x: 335 runs 335 :white_check_mark: 0 :zzz: 0 :x:

Results for commit d924fdc4.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 2 months ago

Code Coverage Summary

Filename             Stmts    Miss  Cover    Missing
-----------------  -------  ------  -------  --------------------------------------------------------------------------------------------------------------------------------------------------------
R/format_value.R       195      12  93.85%   88, 105-112, 187, 296, 396, 407, 415
R/generics.R           118      10  91.53%   255-259, 461, 473, 504, 534, 642, 666-673
R/labels.R              55       7  87.27%   51, 57, 66, 107, 133, 142, 146
R/matrix_form.R        635      42  93.39%   102, 431-432, 511, 523-526, 544, 575, 664-665, 679-684, 714-717, 746-747, 777-778, 860-861, 888, 916, 967, 1123, 1159, 1162-1166, 1213, 1261, 1264, 1268
R/mpf_exporters.R      263      19  92.78%   2, 96-98, 143, 179, 220, 225, 402-408, 412, 415, 453, 546-547
R/page_size.R           45       1  97.78%   171
R/pagination.R         677      38  94.39%   300-303, 404-417, 502, 577, 768-769, 789-798, 990-991, 1175, 1181, 1260, 1390-1391, 1403-1404, 1418-1419
R/tostring.R           586      49  91.64%   40, 92, 162, 195, 203, 239, 296-299, 380-384, 387-390, 397-402, 479, 618-619, 684-691, 740-744, 828, 843, 936, 969, 1010, 1051, 1105, 1112
R/utils.R                3       0  100.00%
R/zzz.R                 17       6  64.71%   28-33
TOTAL                 2594     184  92.91%

Diff against main

Filename          Stmts    Miss  Cover
--------------  -------  ------  -------
R/pagination.R       +1       0  +0.01%
TOTAL                +1       0  +0.00%

Results for commit: d924fdc4dbc06e5c6d92bfde2000af903612acfe

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

edelarua commented 2 months ago

@BFalquet this should fix your issue if you want to test it out. I tested it with the 2 examples you provided and got the correct output.

Note that for the second example you provided I believe you forgot to put the page_num specification within the export_as_txt call, which was part of the issue, but the output is correct if this is fixed.

edelarua commented 2 months ago

Could a unit test be added as well?

A unit test can’t be added in formatters since I wasn’t able to replicate the bug without using rtables/chevron example code, but one could be made elsewhere if needed.

shajoezhu commented 2 months ago

Could a unit test be added as well?

A unit test can’t be added in formatters since I wasn’t able to replicate the bug without using rtables/chevron example code, but one could be made elsewhere if needed.

can we please add a test into scda.test, to catch this downstream

Melkiades commented 2 months ago

For tests, I would ask @BFalquet to add them into {chevron} if he has bandwidth

insightsengineering / formatters