RS-21803: Sig Leader Circles test - #66
Open
SurreyHughesDisplayr wants to merge 4 commits into
Open
Conversation
Cover the sig.leader.circles branch of CreateCustomTable: no-op default, base and filled circle CSS classes, per-cell circle div wrapping, content preservation, circle.size-driven geometry, and the current (buggy) out-of-range normalisation behaviour. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ertions Address review findings on the sig.leader.circles tests: assert the full 9-way circle.colors/circle.border mapping instead of a spot-check, anchor the circle.size geometry assertion to the exact circle CSS declaration (fixed = TRUE) instead of loose token matches, make the "text survives wrapping" test genuinely exercise text preservation with entity content, switch CSS-only fixtures to a value-independent matrix with an explanatory comment, add info= to loop assertions, rename helpers to camelCase to match package style, and reference RS-21803 on the pinned-buggy-behaviour test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Assert the divs half of the "no circle CSS or divs" title, pin full base circle-class declarations with occurrence counts, express the cell-count invariant via prod(dim(x22)), assert an in-range code still works in the out-of-range test, correct the ticket attribution for the unfiled negation-bug comment, and relocate helper definitions to the top of the file per repo convention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gregexpr returns -1L with attributes on no-match, so identical(m, -1L)
was always FALSE and the helper returned 1 for absent patterns,
making every "exactly once" assertion in this file vacuous. Compare
m[1] instead. Also corrected a copy-pasted comment on the nine-variant
loop that incorrectly claimed the trailing " {" is load-bearing there.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds unit test coverage for the
sig.leader.circlesbranch ofCreateCustomTable(R/createcustomtable.R), which had no tests anywhere intests/testthat/..circle2leader,.circle1tie,.circle0) and all nine filled variants (circle21 circle11 circle01 circle20 circle10 circle00 circle2-1 circle1-1 circle0-1) as exact full CSS declarations, so the up/nothing/down fill mapping and the border cycle are both asserted per variant.circle.sizedrives the emitted geometry (line-height/border-radius/height/width) via one anchored declaration.<div class="circleN">(count pinned toprod(dim(x))), and that cell text — including escaped entities — survives the wrapping.sig.leader.circlesisNULL.Pinned production bug
One test deliberately pins current buggy behaviour. The normalisation line in the
sig.leader.circlesbranch iswhich()returns positive indices, so!which(...)is always all-FALSEand the assignment is a complete no-op — out-of-range codes are never coerced to 0, and<div class="circle5">/<div class="circle-3">are emitted instead ofcircle0. The test asserts the actual behaviour, not the documented intent, so it will fail loudly when the bug is fixed. No production code is changed in this PR. No defect ticket is filed for it yet.Jira: https://numbers.atlassian.net/browse/RS-21803
Test plan
46 passing, 0 failures (11
test_thatblocks; 8 added here, plus hardening of the helper used by the pre-existing ones).🤖 Generated with Claude Code