Replace `testify` assertions with custom testing helpers by emilien-puget · Pull Request #159 · stretchr/objx

emilien-puget · 2025-09-10T08:08:28Z

Summary

Replace testify assertions with custom testing helpers

Checklist

Tests are passing: task test
Code style is correct: task lint

emilien-puget · 2025-09-10T08:09:27Z

@ccoVeille wdyt ?

ccoVeille

Great idea @emilien-puget !!

cc @dolmen @brackendawson

Some remarks anyway

ccoVeille · 2025-09-10T08:15:29Z

+// Allow formatting helpers similar to fmt in tests using fmt.Sprintf
+var _ = fmt.Sprintf
+var _ = runtime.NumCPU


I don't see a need for this.

I feel like you are forcing import. So symbols are available.

Why that ?

I'm not saying you are wrong. I'm saying I'm suspicious

ccoVeille · 2025-09-10T08:17:33Z

+var assert nonfatal
+var require fatal


This idea is awesome !

I was like: WTF why removing testify imports everywhere. It won't work!

You got me, and I love the solution you found.

ccoVeille · 2025-09-10T08:20:52Z

+	}
+	return true
+}
+func (a fatal) Equal(t *testing.T, expected, actual interface{}, msgAndArgs ...interface{}) bool {


There is an issue with all fatal methods.

Here I'm talking about a compatibility issue with testify that is pointless as you remove testify dependency.

But I have to mention it.

testify/require asserters don't return bool

So here (and everywhere) it should be this

Suggested change

func (a fatal) Equal(t *testing.T, expected, actual interface{}, msgAndArgs ...interface{}) bool {

func (a fatal) Equal(t *testing.T, expected, actual interface{}, msgAndArgs ...interface{}) {

ccoVeille · 2025-09-10T08:22:04Z

+		msg = fmt.Sprintf(msg, msgAndArgs...)
+	}
+	t.Helper()
+	t.Errorf(msg)


Suggested change

t.Errorf(msg)

t.Error(msg)

ccoVeille · 2025-09-10T08:22:13Z

+		msg = fmt.Sprintf(msg, msgAndArgs...)
+	}
+	t.Helper()
+	t.Fatalf(msg)


Suggested change

t.Fatalf(msg)

t.Fatal(msg)

emilien-puget · 2025-09-10T08:58:02Z

Great idea @emilien-puget !!

cc @dolmen @brackendawson

Some remarks anyway

thanks for the review, i made changes based on your remarks

ccoVeille · 2025-09-10T09:05:25Z

+func (nonfatal) fail(t *testing.T, msg string, msgAndArgs ...interface{}) bool {
+	if len(msgAndArgs) > 0 {
+		msg = fmt.Sprintf(msg, msgAndArgs...)
+	}
+	t.Helper()
+	t.Error(msg)
+	return false
+}
+
+func (fatal) fail(t *testing.T, msg string, msgAndArgs ...interface{}) {
+	if len(msgAndArgs) > 0 {
+		msg = fmt.Sprintf(msg, msgAndArgs...)
+	}
+	t.Helper()
+	t.Fatal(msg)
+}


Nitpicking here.

Usually t.Helper() call in on the first line of a method.

Suggested change

func (nonfatal) fail(t *testing.T, msg string, msgAndArgs ...interface{}) bool {

if len(msgAndArgs) > 0 {

msg = fmt.Sprintf(msg, msgAndArgs...)

}

t.Helper()

t.Error(msg)

return false

}

func (fatal) fail(t *testing.T, msg string, msgAndArgs ...interface{}) {

if len(msgAndArgs) > 0 {

msg = fmt.Sprintf(msg, msgAndArgs...)

}

t.Helper()

t.Fatal(msg)

}

func (nonfatal) fail(t *testing.T, msg string, msgAndArgs ...interface{}) bool {

t.Helper()

if len(msgAndArgs) > 0 {

msg = fmt.Sprintf(msg, msgAndArgs...)

}

t.Error(msg)

return false

}

func (fatal) fail(t *testing.T, msg string, msgAndArgs ...interface{}) {

t.Helper()

if len(msgAndArgs) > 0 {

msg = fmt.Sprintf(msg, msgAndArgs...)

}

t.Fatal(msg)

}

does it changes something or is it more an idiomatic way of doing it ?

It's clearly idiomatic, but for good reasons.

here you have calls to fmt.Sprintf which cannot call t.Error or t.Fail, so it would be OK for now.

But anyone who could come after you to update these lines, may add something that calls t.Error before the call to t.Helper a few line later. Then the line calling t.Errir would be the one reported.

Also, I thought about a possible panic, that might affect the way a test helper could react when something panic.
But I'm unsure how it would affect a test, assuming the panic is caught.

Anyway, fmt.Sprintf handles panics: an example could be a fmt.Stringer that would panic would be represented with PANIC: when passed as %s/%v to fmt.Sprintf

Do, yes. It's idiomatic and safer.

emilien-puget · 2025-09-15T16:31:24Z

@hanzei wdyt ?

hanzei · 2025-09-23T11:28:26Z

Let me fix the CI issues first, and then I'll take a look.

hanzei

I'm not too happy that we are losing some of the nicely styled output of testify with these changes, but I see the need to move away from testify.

LGTM 👍

hanzei · 2025-09-23T11:55:11Z

@emilien-puget @ccoVeille Are we good to merge the PR?

dolmen · 2025-09-26T12:44:05Z

@@ -0,0 +1,238 @@
+package objx_test


Add a comment that mentions why this file exists: this is a reimplementation of a subset of Testify's API to break a cycle dependency with Testify.

i added some words about that

hanzei · 2025-10-03T06:41:57Z

Thank you very much @emilien-puget 🎉

emilien-puget · 2025-10-06T19:54:29Z

@hanzei could we have a release/tag so we can update it on testify side ?

hanzei · 2025-10-07T09:07:37Z

@emilien-puget Done! https://github.com/stretchr/objx/releases/tag/v0.5.3

ccoVeille · 2026-02-24T19:31:57Z

For record, I suggested @alexandear to use your code in @chavacava codeberg project

https://codeberg.org/chavacava/garif/pulls/15#issuecomment-10961225

He liked your idea

stretchr/objx#159 Co-Authored-by: Emilien Puget <[email protected]>

ccoVeille reviewed Sep 10, 2025

View reviewed changes

emilien-puget force-pushed the remove-testify branch from 30daf59 to a4359ce Compare September 10, 2025 08:56

ccoVeille approved these changes Sep 10, 2025

View reviewed changes

emilien-puget force-pushed the remove-testify branch from a4359ce to 374dc22 Compare September 10, 2025 09:21

ccoVeille approved these changes Sep 10, 2025

View reviewed changes

emilien-puget mentioned this pull request Sep 12, 2025

Do we need objx dependency? stretchr/testify#1752

Closed

hanzei reviewed Sep 23, 2025

View reviewed changes

hanzei approved these changes Sep 23, 2025

View reviewed changes

dolmen suggested changes Sep 26, 2025

View reviewed changes

Replace testify assertions with custom testing helpers

f0057b2

emilien-puget force-pushed the remove-testify branch from 8b61542 to f0057b2 Compare October 1, 2025 08:58

Merge branch 'master' into remove-testify

d7420dc

hanzei merged commit b152998 into stretchr:master Oct 3, 2025
6 checks passed

ccoVeille mentioned this pull request Nov 15, 2025

deps: bump objx to v0.5.3 and remove dependency cycle issue stretchr/testify#1823

Merged

ccoVeille mentioned this pull request Feb 24, 2026

Import everything from Emilien Puget idea ccoVeille/testifail#1

Closed

ccoVeille added a commit to ccoVeille/testifail that referenced this pull request Feb 24, 2026

Import Emilien Puget version as-is from

9198879

stretchr/objx#159 Co-Authored-by: Emilien Puget <[email protected]>

ccoVeille added a commit to ccoVeille/testifail that referenced this pull request Feb 24, 2026

Import Emilien Puget version as-is from

cb06f83

stretchr/objx#159 Co-Authored-by: Emilien Puget <[email protected]>

ccoVeille mentioned this pull request Feb 24, 2026

feat: bootstrap testifail Go module with core assertion logic ccoVeille/testifail#3

Merged

ccoVeille added a commit to ccoVeille/testifail that referenced this pull request Feb 24, 2026

Import Emilien Puget version as-is from

2fb8886

stretchr/objx#159 Co-Authored-by: Emilien Puget <[email protected]>

	func (a fatal) Equal(t *testing.T, expected, actual interface{}, msgAndArgs ...interface{}) bool {
	func (a fatal) Equal(t *testing.T, expected, actual interface{}, msgAndArgs ...interface{}) {

Uh oh!

Conversation

emilien-puget commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

emilien-puget commented Sep 10, 2025

Uh oh!

ccoVeille left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilien-puget commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccoVeille Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilien-puget commented Sep 15, 2025

Uh oh!

hanzei commented Sep 23, 2025

Uh oh!

hanzei left a comment

Choose a reason for hiding this comment

Uh oh!

hanzei commented Sep 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hanzei commented Oct 3, 2025

Uh oh!

emilien-puget commented Oct 6, 2025

Uh oh!

hanzei commented Oct 7, 2025

Uh oh!

ccoVeille commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

emilien-puget commented Sep 10, 2025 •

edited

Loading

emilien-puget commented Sep 10, 2025 •

edited

Loading

ccoVeille Sep 10, 2025 •

edited

Loading