test(sanitize): drop the optimization scaffolding - #3121
Merged
SamMorrowDrums merged 1 commit intoAug 19, 2026
Merged
Conversation
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
The benchmarks and the reference-implementation equivalence harness existed to justify the sanitizer rewrite. They have served that purpose, so remove them along with the verbatim copy of the old pipeline they carried. Five checks move into sanitize_test.go rather than going away, because none of them reference the old implementation and all of them guard behaviour the rewrite introduced: - isHTMLInert must be a fixed point of the live bluemonday policy, checked byte by byte and as whole strings, with the accepted byte set pinned explicitly. Nothing else fails if that set is widened, and widening it changes sanitizer output. - Both filters are fixed points on their own output, which is what licenses Sanitize to skip its second pass. - Clean ASCII sanitizes with zero allocations. - Invalid UTF-8 is re-encoded to U+FFFD. - Known payloads still lose content. Net -560 lines. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
SamMorrowDrums
force-pushed
the
sammorrowdrums-drop-sanitizer-migration-scaffolding
branch
from
a2a9761 to
3e9d347
Compare
August 19, 2026 14:41
SamMorrowDrums
deleted the
sammorrowdrums-drop-sanitizer-migration-scaffolding
branch
August 19, 2026 14:42
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Pull request overview
Removes temporary sanitizer optimization benchmarks and equivalence scaffolding while retaining durable regression checks.
Changes:
- Deletes sanitizer and converter benchmarks.
- Removes the legacy reference-equivalence harness.
- Moves essential sanitizer invariants into the main test suite.
| File | Description |
|---|---|
| pkg/sanitize/sanitize_test.go | Adds retained invariant and regression tests. |
| pkg/sanitize/equivalence_test.go | Removes legacy equivalence scaffolding. |
| pkg/sanitize/bench_test.go | Removes sanitizer benchmarks. |
| pkg/github/minimal_types_bench_test.go | Removes converter benchmarks. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
Sorry, something went wrong.
Comment thread
pkg/sanitize/sanitize_test.go
|
|
||
| // TestFiltersAreIdempotent states the fixed-point properties that let Sanitize | ||
| // skip its second pass when HTML normalization changed nothing. | ||
| func TestFiltersAreIdempotent(t *testing.T) { |
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
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.
Follow-up to #3120.
The benchmarks and the equivalence harness existed to justify the sanitizer rewrite and produce its before/after report. That job is done, so this removes them, including the verbatim copy of the old pipeline the harness carried:
pkg/sanitize/sanitize.go is untouched. Net −560 lines.
What is deliberately kept
Five checks move into sanitize_test.go instead of going away. None of them reference the old implementation, so none are equivalence scaffolding, and each guards behaviour the rewrite introduced:
FuzzHTMLInertIsPolicyFixedPoint is kept for the same reason as the first row; the reference-diffing fuzz target is removed. Happy to strip these too if you'd rather have a clean sweep.
Validation
script/lint clean, script/test (race) green, 40s fuzz on the retained target clean.