Repository navigation
Document guidelines for reviewing proposed additions to the API for v2.0.0 #1089
Description
Testify frequently sees requests for new assertions, and we are trying to make the API more consistent in v2.0.0. It's not always clear what should be included (as evidenced by some of the more clunky functions already in the API). I propose that we should write as a wiki page or maybe a markdown file somewhere; the agreed standards which an assertion should meet before it is included in the API.
I've written a draft. It is only my own opinions so please do critique it rigorously. I expect there are good assertions in the API now that contravene some of these ideas.
Proposed Assertion Guidelines
This document is a set of guidelines for the inclusion of the existing and any new assertion functions in the v2.0.0 release.
Language
The function name and argument list should lead to use which documents the intended assertion in reasonably good but terse English.
Good ✅
assert.Equal(t, got, 5)
assert.Between(t, got, 2, 10)Unclear ❌
assert.InDelta(t, got, 6, 4)Argument order
As #399 states, the order of arguments should be:
TestingT- The actual result being tested if distinct from the expectation, or the logical plain English order.
- The expectation being asserted, or the plain English order.
msgAndArgs ...interface{}
Eg
Equal(t TestingT, actual, expected interface{}, msgAndArgs ...interface{}) bool
Contains(t TestingT, s, contains interface{}, msgAndArgs ...interface{}) bool
Less(t TestingT, e1, e2 interface{}, msgAndArgs ...interface{}) boolFormat functions (Assertionf(t TestingT, actual, expected, msg string, args ...interface{})) should not be accepted as they are equivalent to their "non-f" counterparts.
Argument Names
If there is a likely actual value and expected value; then actual and expected should be the argument names.
Equal(t TestingT, actual, expected interface{}, msgAndArgs ...interface{}) boolIf there is no obvious actual value and there are multiple values of the same type; then consistently use v1, v2, ... to differentiate the vs from any other arguments that might be present.
Less(t TestingT, v1, v2 interface{}, msgAndArgs ...interface{}) boolIf there is no obvious actual value and there is only one value, or values of different types; then take example from the standard library or use plain english. Be careful not to follow the stdlib examples if their argument type is descriptive but the assertion uses interface{}.
// error is frequently called err
NoError(t TestingT, err error, msgAndArgs ...interface{}) bool
// sort.SearchInt(a []int, x int) uses a for the slice and x for the element, but the types are critical so this example is bad.
// strings.Contains(s, substr string) is a better example, but s is short for string.
// Just use English or well known abbreviated English for both arguments.
Contains(t, container, val interface{}, msgAndArgs ...interface{}) boolMeta Assertions
A wrapper around an existing assertion should exist only if it improves on the readability of a pure Go implementation.
Useful ✅
assert.ElementsMatch(t, got, expected)visited := make([]bool, len(got))
// - vs -
visited := make([]bool, len(got))
for i := 0; i < len(expected); i++ {
found := false
for j := 0; j < len(got); j++ {
if visited[j] {
continue
}
if got[j] == expected[i] {
visited[j] = true
found = true
break
}
}
if !found {
t.Error("Elements do not match")
}
}
for j := 0; j < len(got); j++ {
if visited[j] {
continue
}
t.Error("Elements do not match")
}Less valuable ❓
assert.InDeltaSlice(t, got, expected)
// - vs -
if assert.Equal(t, len(got), len(expected)) {
for i := range got {
assert.InDelta(t, got[i], expected[i], 5)
}
}Inverse Assertions
I don't know whether to do anything here, it's tempting to say that all inverted functions should be prefixed with "Not", eg: Equal() and NotEqual, But NoError() is perhaps better Language than NotError() and Less() is definitely better than NotGreater().
IsNonDecreasing() however is terrible, every assertion could be prefixed with "Is".
Activity
Antonboom commentedon Aug 28, 2023
I think it would be better to just make a cleaner and stricter API:
like
And
go vetwill be happy 🙂dolmen commentedon Nov 15, 2023
A maintainer's manifesto about Testify
v2A
v2would please users ofv2because of a cleaner API.A
v2would please maintainers ofv2who are fed-up with flaws inv1's API (traps).But
v2will be of no help for users ofv1because that will require migration (even just adding/v2to every import would be a significant and painful migration). With migration (changing code that works) comes a risk of regression. Testify is a set of frameworks for writing testsuites. A testsuite is one critical piece of a software project to protect against regression. The last thing someone wants is a regression in the testsuite that is supposed to protect against regressions.So users of
v1who can't afford migration (rewrite cost, regression risk...) will just stay withv1. Ifv1is abandoned by Testify maintainers who moved tov2,v1users will just be orphaned.Testify is very old and has a ton of users that rely on it. At the time of this post (2023-11-15) pkg.go.dev counts 12,454 public packages dependending on it. But this is only the tip of the iceberg as this doesn't count entreprise code hidden behind walls.
To not orphan users of
v1,v1would have to be maintained along withv2. So what is the benefit for maintainers if they have to maintain bothv1andv2? None: only more work while maintenance resources forv1is already on shortage.So, as the current only active maintainer, I'm declaring that
v2will never happen. Or at least av2of thegithub.com/stretchr/testifymodule with such major breaking changes.Anyone is free to fork the project under a new module name, using changes proposed here or elsewhere, keeping the Testify spirit. This will let users migrate at their own pace. As a maintainer of Testify I will even gladly bless such a fork(s) by linking from testify's documentation (godoc, README). But there is no sane reason for this to happen with the
github.com/stretchr/testifymodule name (the only reason would be branding, but I reject it as a sane reason). Anyway, with how Go modules work, av2would already be a module name change. So proponents of breaking changes must just recognize this and just use a fully separate module name.In this repo we will just continue to maintain
v1.Forever.
Olivier Mengué (@dolmen), current only active maintainer of Testify.
PS: @Antonboom is doing an amazing work with
testifylint. That is a major tool that helps Testify users to avoidv1's traps. More than av2.