test: cover ProfanityFilter edge cases (#940) - #1100
Open
lily1c wants to merge 1 commit into
Open
Conversation
Contributor
🔒 Claude Code Review StatusThank you for your contribution! This PR is from an external repository, so automated Claude review is disabled for security reasons. For maintainers: To get Claude review for this PR, comment PR Summary:
|
Author
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.
What does this PR do?
Adds edge-case tests for the
ProfanityFilterguardrail.While looking at #940 I found that
ProfanityFilteralready has coverage — itlives in
SimpleValidatorSpec.scalain the same package rather than astandalone
ProfanityFilterSpec.scala. (Same reason the issue's pointer toLengthCheckSpec.scala404s:LengthCheckis covered in that combined filetoo.) Rather than duplicate the file, this adds the cases that weren't yet
covered:
caseSensitivemode combined withcustomBadWords— previously untested togetherwithCustomWords(Set.empty)falls back to default behavioursplit("\\s+")handles these, but it wasn't asserted)Known limitation documented, not fixed:
validatesplits on whitespaceonly, so
"badword."and"badword,"don't match the word list. I've added atest asserting current behaviour and labelled it a known limitation rather than
silently changing it. Happy to follow up with a fix (stripping punctuation, or a
word-boundary regex) if that's the preferred direction — it seemed like a
behaviour change that should be a maintainer call rather than folded into a
test-only PR.
Two items in #940's acceptance criteria also conflict with the implementation:
obfuscated variants (
"F*CK","f.u.c.k") aren't detectable given theexact-token matching and two-word default list, and the error message
deliberately withholds the matched word for privacy — there's an existing test
asserting that.
Related issue
Relates to #940
How was this tested?
sbt "core/testOnly *SimpleValidatorSpec"— passessbt scalafmtAll— applied, no diffsbt test— full suite passesChecklist
sbt scalafmtAll— code is formattedsbt test— tests pass on Scala 3main, not from another PR)CHANGELOG.mdunder[Unreleased]— N/A, tests only