-
Notifications
You must be signed in to change notification settings - Fork 1.7k
docs(assert/require): clarify allowed rx values for Regexp/NotRegexp (#268, #1793) #1817
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
8014355
039a069
0aca0d7
5c95c10
0908722
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -281,15 +281,42 @@ func (f *testFunc) ForwardedParamsFormat() string { | |||||
| } | ||||||
|
|
||||||
| func (f *testFunc) Comment() string { | ||||||
| return "// " + strings.Replace(strings.TrimSpace(f.DocInfo.Doc), "\n", "\n// ", -1) | ||||||
| // Preserve original indentation, but ensure lines that start with a tab are | ||||||
| // prefixed with "//\t" (no extra space) to match canonical formatting of code examples. | ||||||
| raw := strings.TrimSpace(f.DocInfo.Doc) | ||||||
| lines := strings.Split(raw, "\n") | ||||||
| for i, line := range lines { | ||||||
| if strings.HasPrefix(line, "\t") { | ||||||
| // Example/code line: keep the leading tab directly after // | ||||||
| lines[i] = "//\t" + strings.TrimPrefix(line, "\t") | ||||||
| } else { | ||||||
| lines[i] = "// " + line | ||||||
| } | ||||||
| } | ||||||
| return strings.Join(lines, "\n") | ||||||
| } | ||||||
|
|
||||||
| func (f *testFunc) CommentFormat() string { | ||||||
| search := fmt.Sprintf("%s", f.DocInfo.Name) | ||||||
| replace := fmt.Sprintf("%sf", f.DocInfo.Name) | ||||||
| comment := strings.Replace(f.Comment(), search, replace, -1) | ||||||
| exp := regexp.MustCompile(replace + `\(((\(\)|[^\n])+)\)`) | ||||||
| return exp.ReplaceAllString(comment, replace+`($1, "error message %s", "formatted")`) | ||||||
| // Start from the original comment text | ||||||
| comment := f.Comment() | ||||||
|
|
||||||
| // 1) Ensure the leading doc header uses the formatted name (e.g., "Equalf asserts ..."). | ||||||
| // Only change the very first line that starts with "// <Name>". | ||||||
| headerPattern := regexp.MustCompile(`^//\s*` + regexp.QuoteMeta(f.DocInfo.Name) + `\b`) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can eliminate this caret to have it replace all of Len to Lenf which is pointed out by this review comment. Besides, https://github.com/stretchr/testify/pull/1817/changes#r2471186618
Suggested change
|
||||||
| comment = headerPattern.ReplaceAllString(comment, "// "+f.DocInfo.Name+"f") | ||||||
|
|
||||||
| // 2) Replace only function call occurrences of the current function name with the formatted variant (suffix 'f'). | ||||||
| // This avoids accidentally changing type names such as "*regexp.Regexp" into "*regexp.Regexpf" by requiring a following '('. | ||||||
| fnCallPattern := regexp.MustCompile(`\b` + regexp.QuoteMeta(f.DocInfo.Name) + `\s*\(`) | ||||||
| comment = fnCallPattern.ReplaceAllString(comment, f.DocInfo.Name+"f(") | ||||||
|
|
||||||
| // 3) Append formatted message args at the end of each Namef(...) call within a single line. | ||||||
| // Use a greedy match until the last closing paren on the same line to avoid inserting inside nested parentheses. | ||||||
| // Example: Namef(t, uint32(123), int32(123)) -> Namef(t, uint32(123), int32(123), "error message %s", "formatted") | ||||||
| addArgsLine := regexp.MustCompile(`(?m)` + regexp.QuoteMeta(f.DocInfo.Name+"f(") + `(.*)\)`) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This multiline mode This can be deleted and would still have the same output from code generation.
Suggested change
For the multiline formatting examples, especially |
||||||
| comment = addArgsLine.ReplaceAllString(comment, f.DocInfo.Name+`f($1, "error message %s", "formatted")`) | ||||||
|
|
||||||
| return comment | ||||||
| } | ||||||
|
|
||||||
| func (f *testFunc) CommentWithoutT(receiver string) string { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -476,7 +476,7 @@ func JSONEqf(t TestingT, expected string, actual string, msg string, args ...int | |
| } | ||
|
|
||
| // Lenf asserts that the specified object has specific length. | ||
| // Lenf also fails if the object has a type that len() not accept. | ||
| // Len also fails if the object has a type that len() not accept. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change seems unexpected
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @ccoVeille, thanks for catching that! The comment was updated to clarify that both [Lenf] and [Len] will fail if the object’s type isn’t accepted by Go’s built-in [len()] function. This makes the documentation more explicit for users who might be unsure about which types are supported. If you think the wording could be improved or if it’s redundant, I’m happy to adjust further. |
||
| // | ||
| // assert.Lenf(t, mySlice, 3, "error message %s", "formatted") | ||
| func Lenf(t TestingT, object interface{}, length int, msg string, args ...interface{}) bool { | ||
|
|
@@ -689,8 +689,14 @@ func NotPanicsf(t TestingT, f PanicTestFunc, msg string, args ...interface{}) bo | |
|
|
||
| // NotRegexpf asserts that a specified regexp does not match a string. | ||
| // | ||
| // The rx (expression) argument should be a *regexp.Regexp. For backward | ||
| // compatibility, if rx is any other type, its value will be formatted with | ||
| // %v and compiled using regexp.Compile. | ||
| // | ||
| // Examples: | ||
| // | ||
| // assert.NotRegexpf(t, regexp.MustCompile("starts"), "it's starting", "error message %s", "formatted") | ||
| // assert.NotRegexpf(t, "^start", "it's not starting", "error message %s", "formatted") | ||
| // assert.NotRegexpf(t, "^start", "it's not starting", "error message %s", "formatted") // string is compiled | ||
| func NotRegexpf(t TestingT, rx interface{}, str interface{}, msg string, args ...interface{}) bool { | ||
| if h, ok := t.(tHelper); ok { | ||
| h.Helper() | ||
|
|
@@ -781,8 +787,14 @@ func Positivef(t TestingT, e interface{}, msg string, args ...interface{}) bool | |
|
|
||
| // Regexpf asserts that a specified regexp matches a string. | ||
| // | ||
| // The rx (expression) argument should be a *regexp.Regexp. For backward | ||
| // compatibility, if rx is any other type, its value will be formatted with | ||
| // %v and compiled using regexp.Compile. | ||
| // | ||
| // Examples: | ||
| // | ||
| // assert.Regexpf(t, regexp.MustCompile("start"), "it's starting", "error message %s", "formatted") | ||
| // assert.Regexpf(t, "start...$", "it's not starting", "error message %s", "formatted") | ||
| // assert.Regexpf(t, "start...$", "it's not starting", "error message %s", "formatted") // string is compiled | ||
| func Regexpf(t TestingT, rx interface{}, str interface{}, msg string, args ...interface{}) bool { | ||
| if h, ok := t.(tHelper); ok { | ||
| h.Helper() | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1708,35 +1708,51 @@ func ErrorContains(t TestingT, theError error, contains string, msgAndArgs ...in | |||||||||||||||||||||
| return true | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // matchRegexp return true if a specified regexp matches a string. | ||||||||||||||||||||||
| func matchRegexp(rx interface{}, str interface{}) bool { | ||||||||||||||||||||||
| // matchRegexp returns whether the provided regular expression matches the input string. | ||||||||||||||||||||||
| // If the rx cannot be compiled into a valid regexp, an error is returned. | ||||||||||||||||||||||
| func matchRegexp(rx interface{}, str interface{}) (bool, error) { | ||||||||||||||||||||||
| var r *regexp.Regexp | ||||||||||||||||||||||
| if rr, ok := rx.(*regexp.Regexp); ok { | ||||||||||||||||||||||
| r = rr | ||||||||||||||||||||||
| } else { | ||||||||||||||||||||||
| r = regexp.MustCompile(fmt.Sprint(rx)) | ||||||||||||||||||||||
| // Safely attempt to compile the pattern; on error, report it so callers can fail the assertion. | ||||||||||||||||||||||
| compiled, err := regexp.Compile(fmt.Sprint(rx)) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| return false, err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| r = compiled | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| switch v := str.(type) { | ||||||||||||||||||||||
| case []byte: | ||||||||||||||||||||||
| return r.Match(v) | ||||||||||||||||||||||
| return r.Match(v), nil | ||||||||||||||||||||||
| case string: | ||||||||||||||||||||||
| return r.MatchString(v) | ||||||||||||||||||||||
| return r.MatchString(v), nil | ||||||||||||||||||||||
| default: | ||||||||||||||||||||||
| return r.MatchString(fmt.Sprint(v)) | ||||||||||||||||||||||
| return r.MatchString(fmt.Sprint(v)), nil | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Regexp asserts that a specified regexp matches a string. | ||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // The rx (expression) argument should be a *regexp.Regexp. For backward | ||||||||||||||||||||||
| // compatibility, if rx is any other type, its value will be formatted with | ||||||||||||||||||||||
| // %v and compiled using regexp.Compile. | ||||||||||||||||||||||
|
Comment on lines
1736
to
+1740
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would use godoc links
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion, @ccoVeille! I’ve updated the doc comments to use GoDoc link syntax for both [regexp.Regexp] and [regexp.Compile] as you recommended. This should make the documentation clearer and more helpful for users. Let me know if you have any other feedback |
||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // Examples: | ||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // assert.Regexp(t, regexp.MustCompile("start"), "it's starting") | ||||||||||||||||||||||
| // assert.Regexp(t, "start...$", "it's not starting") | ||||||||||||||||||||||
| // assert.Regexp(t, "start...$", "it's not starting") // string is compiled | ||||||||||||||||||||||
| func Regexp(t TestingT, rx interface{}, str interface{}, msgAndArgs ...interface{}) bool { | ||||||||||||||||||||||
| if h, ok := t.(tHelper); ok { | ||||||||||||||||||||||
| h.Helper() | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| match := matchRegexp(rx, str) | ||||||||||||||||||||||
| match, err := matchRegexp(rx, str) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| Fail(t, fmt.Sprintf("Invalid regular expression: %v", err), msgAndArgs...) | ||||||||||||||||||||||
| return false | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if !match { | ||||||||||||||||||||||
| Fail(t, fmt.Sprintf("Expect \"%v\" to match \"%v\"", str, rx), msgAndArgs...) | ||||||||||||||||||||||
|
|
@@ -1747,13 +1763,23 @@ func Regexp(t TestingT, rx interface{}, str interface{}, msgAndArgs ...interface | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| // NotRegexp asserts that a specified regexp does not match a string. | ||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // The rx (expression) argument should be a *regexp.Regexp. For backward | ||||||||||||||||||||||
| // compatibility, if rx is any other type, its value will be formatted with | ||||||||||||||||||||||
| // %v and compiled using regexp.Compile. | ||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // Examples: | ||||||||||||||||||||||
| // | ||||||||||||||||||||||
| // assert.NotRegexp(t, regexp.MustCompile("starts"), "it's starting") | ||||||||||||||||||||||
| // assert.NotRegexp(t, "^start", "it's not starting") | ||||||||||||||||||||||
| // assert.NotRegexp(t, "^start", "it's not starting") // string is compiled | ||||||||||||||||||||||
| func NotRegexp(t TestingT, rx interface{}, str interface{}, msgAndArgs ...interface{}) bool { | ||||||||||||||||||||||
| if h, ok := t.(tHelper); ok { | ||||||||||||||||||||||
| h.Helper() | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| match := matchRegexp(rx, str) | ||||||||||||||||||||||
| match, err := matchRegexp(rx, str) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| Fail(t, fmt.Sprintf("Invalid regular expression: %v", err), msgAndArgs...) | ||||||||||||||||||||||
| return false | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if match { | ||||||||||||||||||||||
| Fail(t, fmt.Sprintf("Expect \"%v\" to NOT match \"%v\"", str, rx), msgAndArgs...) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package assert | ||
|
|
||
| import "testing" | ||
|
|
||
| // Verifies that invalid patterns no longer cause a panic when using Regexp/NotRegexp. | ||
| // Instead, the assertion should fail and return false. | ||
| func TestRegexp_InvalidPattern_NoPanic(t *testing.T) { | ||
| NotPanics(t, func() { | ||
| mockT := new(testing.T) | ||
| False(t, Regexp(mockT, "\\C", "whatever")) | ||
| }) | ||
| } | ||
|
|
||
| func TestNotRegexp_InvalidPattern_NoPanic(t *testing.T) { | ||
| NotPanics(t, func() { | ||
| mockT := new(testing.T) | ||
| False(t, NotRegexp(mockT, "\\C", "whatever")) | ||
| }) | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the point of changing here. I tried revert this diff and still got the same output from codegen... 🤔