Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Discussion options

Note

Since writing this proposal I've come to agree with most testify maintainers that v2 shouldn't be made.

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.

  1. Language
  2. Argument Order
  3. Argument Names
  4. Meta Assertions
  5. Inverse Assertions

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:

  1. TestingT
  2. The actual result being tested if distinct from the expectation, or the logical plain English order.
  3. The expectation being asserted, or the plain English order.
  4. 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{}) bool

Format 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{}) bool

If 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{}) bool

If 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{}) bool

Meta 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".

You must be logged in to vote

Replies: 4 comments · 4 replies

Comment options

Format functions (Assertionf(t TestingT, actual, expected, msg string, args ...interface{})) should not be accepted as they are equivalent to their "non-f" counterparts.

I think it would be better to just make a cleaner and stricter API:

Equal(t TestingT, actual, expected any) bool       // No msgAndArgs!
Equalf(t TestingT, actual, expected any, msg string, args ...interface{}) bool

like

package fmt

Print(a ...any) (n int, err error)
Printf(format string, a ...any) (n int, err error)

And go vet will be happy 🙂

You must be logged in to vote
0 replies
Comment options

A maintainer's manifesto about Testify v2

A v2 would please users of v2 because of a cleaner API.
A v2 would please maintainers of v2 who are fed-up with flaws in v1's API (traps).

But v2 will be of no help for users of v1 because that will require migration (even just adding /v2 to 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 v1 who can't afford migration (rewrite cost, regression risk...) will just stay with v1. If v1 is abandoned by Testify maintainers who moved to v2, v1 users 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, v1 would have to be maintained along with v2. So what is the benefit for maintainers if they have to maintain both v1 and v2? None: only more work while maintenance resources for v1 is already on shortage.

So, as the current only active maintainer, I'm declaring that v2 will never happen. Or at least a v2 of the github.com/stretchr/testify module 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/testify module name (the only reason would be branding, but I reject it as a sane reason). Anyway, with how Go modules work, a v2 would 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 avoid v1's traps. More than a v2.

You must be logged in to vote
3 replies
@brackendawson
Comment options

brackendawson Mar 3, 2024
Maintainer Author

That from a Go modules perspective github.com/stretchr/testify/v2 is functionally the same as absolutely.anywhere/else/testify is insightful, thank you for pointing that out.

For sure if a v2 were made here and v1 not maintained forever then this would kind of split the testify community, certainly make life harder for the users. v1 must be maintained forever.

On the other hand if maintainers are happy to maintain both releases then them sharing an issue backlog is useful. The "forks" would diverge less. As one of the five maintainers, of which I think three are active, I am happy to do this. A v2 can be prototyped without impacting v1 users at all, it can even be abandoned after a BETA release without them knowing it happened.

@dolmen
Comment options

@brackendawson A v2 would be developed on a separate branch than v1. That will require posting fixes to both branches, and this is a process that is not handled by GitHub's UI, so much more manual work for maintainers.

@fredbi
Comment options

@brackendawson i like a lot all the prescribed principles above.

I think that an important additional constraint to be added is to avoid adding dependencies to external packages, or at least not external packages from other organizations (so we may easily « bless » dependencies from stretchr).

Bonus features that want dependencies may be enabled voluntarily by consumers who explicitly enable it by a specific import.

Ready to help there if needed, as i’ve been through the process of reducing the dependencies footprint of the go-openapi repositories.

cheers

fred

Comment options

@brackendawson Hi! I wanted to share that I've reached a stable release (v2.4.0) of the go-openapi fork of testify at https://github.com/go-openapi/testify — an
exploration of what a v2 could look like.

It's ~90% compatible with the original and follows the same fundamental principles. The experimental phase is now over: the API is stable and actively consumed by
go-openapi/go-swagger.

Some highlights:

  • Zero external dependencies (spew/difflib internalized)
  • Code generation from a single source of truth (127 assertions generating 840 functions)
  • Generic assertion variants
  • New assertions: Eventually/Consistently (with context), NoFileDescriptorLeak, and others
  • Documentation site: https://go-openapi.github.io/testify/

I know @ccoVeille has been monitoring and comparing a few things to bring back to the original — I'd be happy to contribute upstream as well if any particular feature or
approach looks like a good fit for stretchr/testify.

Cheers,
Fred (@fredbi)

You must be logged in to vote
0 replies
Comment options

Please consider //go:fix inline, so users can easily migrate to v2 when it is ready for release.

Also... is there a reason this package uses interface{} instead of the handy (and in my opinion, more readable) any alias?
Maybe ignore the last part, as Go is not my primary language.

You must be logged in to vote
1 reply
@fredbi
Comment options

Please consider //go:fix inline, so users can easily migrate to v2 when it is ready for release.

Also... is there a reason this package uses interface{} instead of the handy (and in my opinion, more readable) any alias? Maybe ignore the last part, as Go is not my primary language.

The README clearly states that no v2 is in sight, I am afraid.

My post above was referring to a hard-forked v2 (released) hopefully with some useful ideas to retrofit in the current v1.

The v2 in the fork comes with a migration tool as //go: fix inline is not really practical for this use case (mostly useful for backward compatible shims).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Category
💡
Ideas
Labels
None yet
5 participants
Converted from issue

This discussion was converted from issue #1089 on March 03, 2024 14:09.

Morty Proxy This is a proxified and sanitized view of the page, visit original site.