Refactor overlapping test coverage for streaming and creative rewrites

[ChristianPavilonis]

Summary

• Refactor overlapping UTF-8 boundary tests in streaming_replacer into shared helpers and consolidated coverage-focused cases.
• Consolidate repetitive srcset and imagesrcset creative rewrite tests into shared helpers plus table-driven case matrices.
• Keep runtime behavior unchanged while reducing duplicated test setup and preserving multibyte, emoji, spacing, relative path, and descriptor coverage.

Changes

File	Change
crates/trusted-server-core/src/streaming_replacer.rs	Added reusable chunk-processing test helpers and replaced overlapping UTF-8 boundary tests with consolidated case-based coverage, including explicit multibyte and emoji boundary splits.
crates/trusted-server-core/src/creative.rs	Added shared srcset/imagesrcset test helpers and merged overlapping attribute rewrite tests into compact table-driven cases while keeping integration-specific tests separate.

Closes

Closes #457
Closes #458

Test plan

• cargo test --workspace
• cargo clippy --workspace --all-targets --all-features -- -D warnings
• cargo fmt --all -- --check
• JS tests: cd crates/js/lib && npx vitest run
• JS format: cd crates/js/lib && npm run format
• Docs format: cd docs && npm run format
• WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve
• Other: Focused Rust test runs for the new consolidated test cases

Checklist

• Changes follow CLAUDE.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses tracing macros (not println!)
• New code has tests
• No secrets or credentials committed

[aram356]

Summary

Pure test refactor consolidating overlapping srcset/imagesrcset attribute tests in creative.rs and UTF-8 boundary tests in streaming_replacer.rs. The 1:1 mapping from old tests to new table-driven cases is faithful, and several imagesrcset cases gain stronger assertions (>= 2 rewritten candidates instead of >= 1). Two issues need attention before merge — see inline comments.

Blocking

🔧 wrench

• Multibyte boundary test still doesn't split mid-character: see inline at streaming_replacer.rs:468. The PR fixes the analogous emoji bug but leaves the multibyte case mis-named.
• Helper assertion message contradicts its check: see inline at creative.rs:798. >= 2 with message expected two should be == 2.

Non-blocking

🤔 thinking

• <div> → <img> element switch in helper: see inline at creative.rs:781. Functionally equivalent given the [imagesrcset] selector, but neither old nor new tests cover the realistic <link rel="preload" as="image" imagesrcset="…"> path. Worth a follow-up issue.
• Loss of cargo test <single-case> granularity: collapsing 8 standalone tests into 2 table-driven tests means a failing inner case can't be re-run in isolation — only the umbrella test. The case_name in assertion messages mitigates this for log-reading, but bisecting harder failures is more friction. Acceptable tradeoff; flagging as a real downside without a parameterized-test macro (rstest or similar).

📝 note

• Test-name surface shrinks: cargo test output loses 6 named tests (8 srcset/imagesrcset → 2 umbrellas, plus the streaming consolidation). Any CI dashboard or flake tracker keyed on test names will need to be aware.

CI Status

• cargo fmt: PASS
• cargo clippy: PASS
• cargo test: PASS
• vitest: PASS
• format-typescript / format-docs: PASS
• integration tests / browser integration tests: PASS

[aram356]

Summary

Pure test refactor consolidating overlapping srcset/imagesrcset and UTF-8 boundary tests. The fix-up commit 4b6586cc correctly resolves both 🔧 findings from the prior review:

1. Mid-multibyte split — streaming_replacer.rs:468 split point moved from byte 23 (UTF-8 boundary, after ø completes) to byte 22 (between 0xC3 and 0xB8, genuinely inside ø).
2. Exact-count assertion — creative.rs:794 changed from assert!(... >= 2) to assert_eq!(..., 2) with matching "expected exactly two" message.

I also confirmed test_process_chunk_boundary_in_emoji (split at byte 2 of 🎉, a 4-byte sequence) genuinely exercises a mid-emoji boundary; the pre-refactor version processed the entire content in a single process_chunk call despite its name. The renames are a real coverage improvement, not just naming.

No coverage was lost in the consolidation — every old assertion (count, descriptors, relative preserved) maps 1:1 to a table case and is preserved or strengthened.

Verified locally: cargo test -p trusted-server-core (creative 75 ok, streaming_replacer 21 ok), cargo clippy --all-targets --all-features -- -D warnings clean, cargo fmt --all -- --check clean.

Non-blocking

♻️ refactor

• Duplicate SrcsetCase struct: creative.rs:989 and creative.rs:1101 define an identical struct and run an identical loop. See inline comment.

🌱 seedling

• Realistic placement for imagesrcset: helper wraps in <img>, which is spec-illegal. Pre-existing pattern; worth a follow-up. See inline comment.

📝 note

• Stale checklist items in PR body: JS tests, JS format, docs format, WASM build are unchecked but don't apply to this Rust-test-only PR. All CI is green. Informational only.

CI Status

• fmt: PASS
• clippy: PASS
• rust tests: PASS
• js tests: PASS
• js format: PASS
• docs format: PASS
• browser integration tests: PASS

[aram356]

♻️ refactor — SrcsetCase<'a> is defined identically here and again at line 1101 in rewrites_imagesrcset_attribute_cases, and both functions run a structurally identical loop differing only in the attribute name passed to the helper.

Two paths to consolidate further:

// Hoist struct to module scope, single test loops over both attrs
struct SrcsetCase<'a> { name: &'a str, attr_value: &'a str, expected_relative: &'a str, expected_descriptors: &'a [&'a str] }

#[test]
fn rewrites_srcset_and_imagesrcset_attribute_cases() {
    for attr in ["srcset", "imagesrcset"] {
        for case in CASES { assert_rewritten_srcset_attr_case(case.name, attr, ...); }
    }
}

Splitting per attribute does give clearer panic messages, so this is a judgment call. Suggestion only — leaving as-is is fine.

[aram356]

🌱 seedling — rewrite_srcset_attr wraps both srcset and imagesrcset in <img>. <img imagesrcset> is HTML-spec-illegal placement (imagesrcset belongs on <link rel="preload" as="image"> or <source>).

This PR does not regress on that — the pre-refactor <div imagesrcset> was equally spec-illegal — and the tests still validate the rewrite logic correctly via the element-agnostic [imagesrcset] selector at line 660. Worth a follow-up issue to add a realistic <source>/<link> test path.