fix(assert): handle invalid regexp gracefully instead of panicking#1854
Open
bk-simon wants to merge 2 commits intostretchr:masterfrom
Open
fix(assert): handle invalid regexp gracefully instead of panicking#1854bk-simon wants to merge 2 commits intostretchr:masterfrom
bk-simon wants to merge 2 commits intostretchr:masterfrom
Conversation
Replace regexp.MustCompile with regexp.Compile in matchRegexp so that an invalid regexp pattern fails the test with a useful error message rather than causing a runtime panic. Fixes stretchr#1794
ccoVeille
reviewed
Feb 25, 2026
There was a problem hiding this comment.
Pull request overview
This PR fixes a panic issue in assert.Regexp and assert.NotRegexp when invalid regexp patterns are provided. Previously, these functions used regexp.MustCompile, which panics on invalid patterns. Now they gracefully fail the test with a descriptive error message.
Changes:
- Modified
matchRegexpto return an error instead of panicking on invalid regexp compilation - Updated
RegexpandNotRegexpto handle compilation errors gracefully - Added regression test to verify invalid patterns fail tests without panicking
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| assert/assertions.go | Changed matchRegexp to return error, replaced regexp.MustCompile with regexp.Compile, added error handling in Regexp and NotRegexp |
| assert/assertions_test.go | Added TestRegexpInvalidExpression to verify invalid patterns fail gracefully |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…xpInvalidExpression Address reviewer feedback: - Switch from new(testing.T) to new(mockTestingT) so we can inspect the actual error message, not just that the test failed - Assert the error string contains "Invalid regexp" for both Regexp and NotRegexp with an invalid pattern - Simplify inline comment: drop historical "previously caused a panic" language since commit history is the right place for that context
ccoVeille
approved these changes
Feb 25, 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
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
Fixes #1794
assert.Regexpandassert.NotRegexpusedregexp.MustCompileinternally (viamatchRegexp) when therxargument was a string or non-*regexp.Regexpvalue.MustCompilepanics on an invalid regexp pattern, which is documented as intended only for package-level initialization of global variables.This causes a runtime panic rather than a test failure when a user accidentally passes an invalid regexp:
What changed
matchRegexpsignature fromfunc matchRegexp(rx, str interface{}) booltofunc matchRegexp(rx, str interface{}) (bool, error)regexp.MustCompilewithregexp.Compile— on error the function returns the compile errorRegexpandNotRegexpnow callFailwith a descriptive"Invalid regexp: ..."message when the pattern fails to compile, so the test fails gracefully rather than panickingTestRegexpInvalidExpressionto guard against regressionTest plan
go test ./assert/ -run TestRegexp— all passgo test ./...— all packages passTestRegexpInvalidExpressionspecifically verifies invalid patterns fail gracefully (not panic)