Skip to content

RS-21803: checkImageTag style attribute test - #72

Open
SurreyHughesDisplayr wants to merge 2 commits into
RS-21803-colwidthsandcustomcssfrom
RS-21803-checkImageTagstyleattribute
Open

RS-21803: checkImageTag style attribute test#72
SurreyHughesDisplayr wants to merge 2 commits into
RS-21803-colwidthsandcustomcssfrom
RS-21803-checkImageTagstyleattribute

Conversation

@SurreyHughesDisplayr

Copy link
Copy Markdown

Summary

Adds unit test coverage for checkImageTag's handling of the style attribute and its surrounding link-cleaning paths (R/checkimagetag.R) — 7 test blocks in the existing test-checkimagetag.R.

  • Covers a style attribute before and after src, exercising the two different quote/bracket cleaning paths rather than the same one twice.
  • Covers double-quoted and unquoted src values, and src followed by trailing text.
  • Covers the missing-src branch, pinning the echoed input in the warning so it is distinguishable from the tag-regex-miss branch that emits a byte-identical syntax error prefix.
  • Asserts the success path wraps the original text, not the matched img tag.
  • Each success block asserts the whole returned string in one expect_equal rather than several substring greps.

Also closes a pre-existing gap: the file's txt.dq case was single-quoted and identical to txt.sq, so double quotes were never actually tested. The style literal is now a single hoisted constant, replacing a dangling unused assignment.

Note on the src= regex

The last block pins current behaviour for a decoy input where an earlier attribute value contains src=. regexpr("src=(\S+)", ...) is unanchored, so alt='mysrc=x' is matched in preference to the real src, and a valid image is silently removed with a user-visible invalid link which has been removed: x warning. The test pins the warning text — deliberately, so that the assertion fails for the decoy and not merely because a network fetch failed, since the same warning is emitted on any failed GET.

This looks like a genuine low-severity defect rather than intended behaviour: nothing in the function or its callers suggests the loose match is deliberate, and a boundary requirement such as (?:^|\s)src= would be safe because the enclosing <img [^>]+> match guarantees whitespace before every attribute. Not fixed here; worth a ticket. Separately, the trailing-whitespace sub() in the same function is unreachable, since (\S+) cannot capture whitespace — inert rather than wrong.

Network dependency

The success-path blocks perform a live HTTP GET of a real wiki image URL — the pre-existing convention in this file, not introduced here. They fail offline. Documented in a comment at the top of the file so the failure mode is obvious to the next reader.

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

Stacked PR

Based on RS-21803-colwidthsandcustomcss (#71). Merge order: #66, #67, #68, #69, #70, #71, then this. This is the last of the seven RS-21803 plans; it is the only one touching checkimagetag.R rather than createcustomtable.R, so it shares no test file with the others.

Test plan

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

24 passing, 0 failures, 0 warnings (requires network access).

🤖 Generated with Claude Code

SurreyHughesDisplayr and others added 2 commits August 21, 2026 14:47
Adds scenarios for style and width attributes preceding src, attribute
order independence, double-quoted and unquoted src values, a missing
src attribute, surrounding text preservation, and a decoy substring
"src=" match that is not silently treated as a valid link.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace unfailable identical() check and network-dependent warning match
in the src= decoy block with pinned real values (result "" and warning
text including the captured "x"); pin echoed input in the missing-src
syntax-error message to distinguish it from the tag-regex-miss branch;
drop redundant duplicate expect_warning call; hoist repeated txt.style
literal to a file-level constant and remove dangling duplicate txt.dq
assignment; rename reused txt.dq in the double-quote block; document
network dependency of success-path blocks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant