RS-21803: Banded Rows test

[SurreyHughesDisplayr]

Summary

Adds unit test coverage for the banded.rows and banded.cols arguments of CreateCustomTable (R/createcustomtable.R).

• Pins each banding rule as one contiguous exact string — both the odd and even clauses and both fill values — so inserting anything between the clauses, or swapping banded.odd.fill with banded.even.fill, fails the test.
• Asserts no nth-child rule is emitted at all when banding is off.
• Pins the column rule verbatim, including the td:nth-child(2n+3) / td:nth-child(even) pair as coded.
• Asserts banded.odd.fill / banded.even.fill reach both emitted rules. These are separate cata() calls that interpolate the arguments independently, so the row branch, the column branch, and both-at-once are each exercised with non-default values.
• Asserts that enabling banding removes the configured cell.fill from the celldefault declaration, so the banded background is what shows. Banding wins over cell.fill, not the other way round (createcustomtable.R:397-400 blanks the cell-fill matrix whenever either banding argument is set). Pinned as a single-variable contrast against the unbanded case, where the same cell.fill value does reach the declaration.
• Asserts the banding rule is emitted once regardless of row count.
• Extracts the generated container id and asserts the first clause is scoped to it, guarded by expect_length(containerSel, 1).

Pinned production defects

Two tests deliberately pin current buggy behaviour and will fail loudly when fixed. Both are ticketed.

1. The second clause of each banding rule is unscoped — RS-23594. Every other rule this function emits is namespaced under the generated container class. The banding rules are the exception: only the first clause carries the prefix, so tr:nth-child(even) / td:nth-child(even) is emitted bare and applies to every table on the page. Affected tables need neither banding enabled nor to be CreateCustomTable output. Because the odd clause is scoped, the two halves split — unrelated tables pick up the banded fill on even rows only, which reads as inconsistent striping rather than an obvious fault.

2. banded.cols leaves the first column unbanded when row headers are hidden — RS-23595. The selectors assume a row-header cell at nth-child(1). With show.row.headers = FALSE the data columns start at position 1, which matches neither 2n+3 ({3,5,…}) nor even ({2,4,…}). Verified: the emitted rule is byte-identical with and without row headers, so the selectors are not header-aware. The test asserts that equality, which is what trips if anyone makes them adapt.

Plan corrections

Two of the plan's expectations did not survive checking and were corrected rather than encoded:

• The plan's example CSS strings have no space before ;. cata() writes via cat(..., sep = " "), so real output is background-color: rgb(250,250,250) ; — the plan's string never occurs. Tests pin the real text.
• The plan records the 2n+3 / even column selectors as overlapping and asks for that to be logged as a defect. They are provably disjoint: 2n+3 = {3, 5, 7, …} and even = {2, 4, …}. With row headers shown the header cell occupies nth-child(1), so data columns land on 2, 3, 4 and alternate correctly. Not a defect — the real defect in that rule is the header-hidden case above, which the plan did not raise.

No production code is changed in this PR.

Jira: https://numbers.atlassian.net/browse/RS-21803

Stacked PR

Based on RS-21803-rowspans (#68), not master — these plans all add tests to tests/testthat/test-createcustomtable.R and share its helpers. #66 has already merged to master, so the remaining merge order is #67, #68, then this. This branch is up to date with its base, and the diff against it is this plan's work only.

Test plan

Rscript -e "devtools::load_all('.'); testthat::test_file('tests/testthat/test-createcustomtable.R')"

188 passing, 0 failures, 0 warnings (48 test_that blocks; 8 added here). Also run under LANGUAGE=fr with identical results.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds characterization tests for row and column banding in CreateCustomTable.

Changes:

• Verifies generated banding CSS, fill behavior, selector scoping, and defaults.
• Covers combined banding and row-count independence.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[SurreyHughesDisplayr]

Confirmed by mutation, and it was a real gap. banded.rows and banded.cols are separate cata() calls (createcustomtable.R:626-631) that each interpolate banded.odd.fill/banded.even.fill independently, and only the row branch was exercised with non-default values.

I hard-coded the defaults in the banded.cols call exactly as you describe:

cata(container.selector.name, 'tbody td:nth-child(2n+3){background-color:', 'rgb(250,250,250)',
     ';} td:nth-child(even){background-color:', 'rgb(245,245,245)', ';}')

The suite stayed fully green at 181 passing. Against the same mutant the extended test now fails on 6 assertions.

Added three cases rather than one: banded.cols alone with rgb(1,1,1)/rgb(2,2,2), and banded.rows + banded.cols together. The both-at-once case matters because the two branches take the same argument pair — a regression that forwarded them in only one of the two rules would otherwise still pass the single-branch tests.

188 passing, 0 failures. Production is unchanged; the mutation was reverted.

[SurreyHughesDisplayr]

You are right, the summary had it backwards. The test pins that enabling banding removes the configured cell.fill, not that cell.fill suppresses the banding.

Confirmed in production (createcustomtable.R:397-400):

if (!banded.rows && !banded.cols)
    cell.fill <- matrix(rep(paste("background:", cell.fill, ";"), length = nrows * ncols), nrows, ncols)
else
    cell.fill <- matrix("", nrows, ncols)

Banding wins: the moment either banding argument is set, the cell-fill matrix is blanked, so the banded background is what shows. Summary corrected to say that, with the line reference and a note that the single-variable contrast against the unbanded case is what makes the assertion meaningful — with banded.rows = FALSE the same cell.fill value does reach the celldefault declaration.

The test names themselves already described the right direction ("banded.rows = TRUE drops the per-cell background from the celldefault CSS"), so only the PR body needed the fix.

[chschan]

LGTM