refactor: Move from testify to be #130

2026-01-07T21:32:06Z

nemith commented

2026-01-07 21:32:06 +00:00

(Migrated from github.com)

testify has a lot of indirect dependencies. Be has zero.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-01-07 21:39:43 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This PR refactors the test suite from testify to the be testing library to reduce indirect dependencies. The migration replaces testify's assertion functions with their be equivalents across all test files, and updates go.mod/go.sum to remove testify and its dependencies.

Migrates all test assertions from testify/assert and testify/require to github.com/carlmjohnson/be
Removes testify and its indirect dependencies from go.mod
Adds new imports for errors and strings packages where needed for manual error checking

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
transport/tls/tls_test.go	Converts assertions from testify to be, adds errors import for error checking
transport/ssh/ssh_test.go	Converts assertions from testify to be, adds errors and strings imports
transport/frame_test.go	Converts assertions, adds string conversions for byte slice comparisons
session_test.go	Converts all session test assertions to be equivalents
rpc/rpc_test.go	Converts RPC test assertions to be
rpc/notification_test.go	Converts notification test assertions to be
rpc/filter_test.go	Converts filter test assertions to be
rpc/config_test.go	Converts config operation test assertions to be
msg_test.go	Converts message marshaling/unmarshaling test assertions to be
inttest/ssh_test.go	Converts integration test assertions to be, adds errors import
go.mod	Replaces testify with be, removes indirect dependencies
go.sum	Updates checksums for new dependency tree

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview This PR refactors the test suite from testify to the `be` testing library to reduce indirect dependencies. The migration replaces testify's assertion functions with their be equivalents across all test files, and updates go.mod/go.sum to remove testify and its dependencies. - Migrates all test assertions from `testify/assert` and `testify/require` to `github.com/carlmjohnson/be` - Removes testify and its indirect dependencies from go.mod - Adds new imports for `errors` and `strings` packages where needed for manual error checking ### Reviewed changes Copilot reviewed 11 out of 12 changed files in this pull request and generated 7 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | transport/tls/tls_test.go | Converts assertions from testify to be, adds errors import for error checking | | transport/ssh/ssh_test.go | Converts assertions from testify to be, adds errors and strings imports | | transport/frame_test.go | Converts assertions, adds string conversions for byte slice comparisons | | session_test.go | Converts all session test assertions to be equivalents | | rpc/rpc_test.go | Converts RPC test assertions to be | | rpc/notification_test.go | Converts notification test assertions to be | | rpc/filter_test.go | Converts filter test assertions to be | | rpc/config_test.go | Converts config operation test assertions to be | | msg_test.go | Converts message marshaling/unmarshaling test assertions to be | | inttest/ssh_test.go | Converts integration test assertions to be, adds errors import | | go.mod | Replaces testify with be, removes indirect dependencies | | go.sum | Updates checksums for new dependency tree | </details> --- 💡 <a href="/nemith/netconf/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

session_test.go

				`@ -98,13 +97,13 @@ func TestWithCapability(t *testing.T) {`

Using be.Equal for comparing false with a boolean result is correct, but it would be more idiomatic to use be.False(t, session.ClientCaps().Has(CapNetConf10)) which better expresses the intent of the assertion.

	be.False(t, session.ClientCaps().Has(CapNetConf10))

Using `be.Equal` for comparing `false` with a boolean result is correct, but it would be more idiomatic to use `be.False(t, session.ClientCaps().Has(CapNetConf10))` which better expresses the intent of the assertion. ```suggestion be.False(t, session.ClientCaps().Has(CapNetConf10)) ```

session_test.go

					
				@ -209,2 +206,4 @@

					be.Nonzero(t, err)

					be.Zero(t, session)

					// Transport should be closed on error

The assertion has changed semantics. Previously, assert.Equal(t, handler, session.notifHandler) checked that the handler is the exact same instance. Now, be.Nonzero(t, session.notifHandler) only checks that the handler is not nil, which is a weaker assertion. Consider keeping the original equality check with be.Equal(t, handler, session.notifHandler) to maintain test coverage.

The assertion has changed semantics. Previously, `assert.Equal(t, handler, session.notifHandler)` checked that the handler is the exact same instance. Now, `be.Nonzero(t, session.notifHandler)` only checks that the handler is not nil, which is a weaker assertion. Consider keeping the original equality check with `be.Equal(t, handler, session.notifHandler)` to maintain test coverage.

session_test.go

					
				@ -254,29 +253,29 @@ func TestPrepareDoesNotMutate(t *testing.T) {

					prepared := session.Prepare(rpc)

The assertion has changed semantics. Previously, assert.Equal(t, handler, session.notifHandler) checked that the handler is the exact same instance. Now, be.Nonzero(t, session.notifHandler) only checks that the handler is not nil, which is a weaker assertion. Consider keeping the original equality check with be.Equal(t, handler, session.notifHandler) to maintain test coverage.

The assertion has changed semantics. Previously, `assert.Equal(t, handler, session.notifHandler)` checked that the handler is the exact same instance. Now, `be.Nonzero(t, session.notifHandler)` only checks that the handler is not nil, which is a weaker assertion. Consider keeping the original equality check with `be.Equal(t, handler, session.notifHandler)` to maintain test coverage.

session_test.go

					
				@ -269,4 +268,4 @@

					rpc := NewRPC("test-op")

					ctx := context.Background()

					msg, err := session.Do(ctx, rpc)

The assertion semantics have changed. assert.NotNil(t, session.reqs) checks if the field is not nil, while be.Equal(t, 0, len(session.reqs)) checks if it's an empty map. These are different assertions - the former tests initialization, the latter tests emptiness. Use be.Nonzero(t, session.reqs) to maintain the original assertion semantics.

The assertion semantics have changed. `assert.NotNil(t, session.reqs)` checks if the field is not nil, while `be.Equal(t, 0, len(session.reqs))` checks if it's an empty map. These are different assertions - the former tests initialization, the latter tests emptiness. Use `be.Nonzero(t, session.reqs)` to maintain the original assertion semantics.

session_test.go

					
				@ -883,12 +882,12 @@ func TestHandshakeNoUpgradeWhenServerLacks11(t *testing.T) {

					ut := &upgradableTransport{Transport: tt}

Using be.Equal for comparing false with a boolean result is correct, but it would be more idiomatic to use be.False(t, ut.upgraded) which better expresses the intent of the assertion.

	be.False(t, ut.upgraded)

Using `be.Equal` for comparing `false` with a boolean result is correct, but it would be more idiomatic to use `be.False(t, ut.upgraded)` which better expresses the intent of the assertion. ```suggestion be.False(t, ut.upgraded) ```

transport/ssh/ssh_test.go

					
				@ -221,8 +223,8 @@ func TestTransport_DialContextCancel(t *testing.T) {

					start := time.Now()

The duration comparison logic is incorrect. time.Since(start).Abs() returns a duration that's always positive, but the check should verify that the elapsed time is less than 200ms. The current code uses .Abs() on a duration, which is not correct. Use time.Since(start) <= 200*time.Millisecond instead.

	be.True(t, time.Since(start) <= 200*time.Millisecond)

The duration comparison logic is incorrect. `time.Since(start).Abs()` returns a duration that's always positive, but the check should verify that the elapsed time is less than 200ms. The current code uses `.Abs()` on a duration, which is not correct. Use `time.Since(start) <= 200*time.Millisecond` instead. ```suggestion be.True(t, time.Since(start) <= 200*time.Millisecond) ```

transport/tls/tls_test.go

					
				@ -163,8 +163,8 @@ func TestTransport_DialContextCancel(t *testing.T) {

					start := time.Now()

The duration comparison logic is incorrect. time.Since(start).Abs() returns a duration that's always positive, but the check should verify that the elapsed time is less than 200ms. The current code uses .Abs() on a duration, which is not correct. Use time.Since(start) <= 200*time.Millisecond instead.

	be.True(t, time.Since(start) <= 200*time.Millisecond)

The duration comparison logic is incorrect. `time.Since(start).Abs()` returns a duration that's always positive, but the check should verify that the elapsed time is less than 200ms. The current code uses `.Abs()` on a duration, which is not correct. Use `time.Since(start) <= 200*time.Millisecond` instead. ```suggestion be.True(t, time.Since(start) <= 200*time.Millisecond) ```

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-01-07 21:44:10 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments. --- 💡 <a href="/nemith/netconf/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

msg_test.go

The test assertion has been weakened. The original assert.Equal(t, got, tc.msg) compared the entire Hello struct for equality, but now only the SessionID and the length of Capabilities are checked. This means that if the Capabilities slice contains different values (but the same length), or if XMLName differs, the test will pass when it should fail. The complete struct comparison should be preserved.

			be.Equal(t, tc.msg, got)

The test assertion has been weakened. The original `assert.Equal(t, got, tc.msg)` compared the entire Hello struct for equality, but now only the SessionID and the length of Capabilities are checked. This means that if the Capabilities slice contains different values (but the same length), or if XMLName differs, the test will pass when it should fail. The complete struct comparison should be preserved. ```suggestion be.Equal(t, tc.msg, got) ```

msg_test.go

The test assertion has been weakened. The original assert.Equal(t, tc.want, got) compared the entire RPCReply struct for equality, but now only the MessageID and the length of RPCErrors are checked. This means that if the RPCErrors slice contains different values (but the same length), or if XMLName differs, the test will pass when it should fail. The complete struct comparison should be preserved.

			be.Equal(t, want, got)

The test assertion has been weakened. The original `assert.Equal(t, tc.want, got)` compared the entire RPCReply struct for equality, but now only the MessageID and the length of RPCErrors are checked. This means that if the RPCErrors slice contains different values (but the same length), or if XMLName differs, the test will pass when it should fail. The complete struct comparison should be preserved. ```suggestion be.Equal(t, want, got) ```

transport/ssh/ssh_test.go

					
				@ -281,3 +283,3 @@

					err = w.Close()

					assert.NoError(t, err)

					be.NilErr(t, err)

The assertion logic has been changed to check both conditions redundantly. The check err != nil && strings.Contains(err.Error(), "unable to authenticate") is unnecessarily verbose. If be.In is the appropriate replacement for assert.ErrorContains, it should be used directly. However, the pattern suggests be.In expects the substring first and the string second, so this should be simplified to just be.In(t, "unable to authenticate", err.Error()) after first checking that err is not nil with be.Nonzero(t, err) on line 191.

The assertion logic has been changed to check both conditions redundantly. The check `err != nil && strings.Contains(err.Error(), "unable to authenticate")` is unnecessarily verbose. If `be.In` is the appropriate replacement for `assert.ErrorContains`, it should be used directly. However, the pattern suggests `be.In` expects the substring first and the string second, so this should be simplified to just `be.In(t, "unable to authenticate", err.Error())` after first checking that err is not nil with `be.Nonzero(t, err)` on line 191.

transport/ssh/ssh_test.go

					
				@ -287,2 +288,3 @@

					be.NilErr(t, err)

					require.NoError(t, srv.Wait(t))

					be.NilErr(t, srv.Wait(t))

The assertion logic has been changed to check both conditions redundantly. The check err != nil && strings.Contains(err.Error(), "no auth passed yet") is unnecessarily verbose. Since be.Nonzero(t, err) was not called before this assertion to verify err is not nil, this could panic if err is nil. This should be split into two separate assertions: first check that err is not nil, then check the error message content.

	be.Nonzero(t, err)
	be.True(t, strings.Contains(err.Error(), "no auth passed yet"))

The assertion logic has been changed to check both conditions redundantly. The check `err != nil && strings.Contains(err.Error(), "no auth passed yet")` is unnecessarily verbose. Since `be.Nonzero(t, err)` was not called before this assertion to verify err is not nil, this could panic if err is nil. This should be split into two separate assertions: first check that err is not nil, then check the error message content. ```suggestion be.Nonzero(t, err) be.True(t, strings.Contains(err.Error(), "no auth passed yet")) ```

Sign in to join this conversation.

No reviewers