refactor(test): perf optimize and log loc correct (cloudwego#1455)

internal/test/assert.go

@@ -27,10 +27,10 @@ type testingTB interface {
 
 // Assert asserts cond is true, otherwise fails the test.
 func Assert(t testingTB, cond bool, val ...interface{}) {
-	t.Helper()
 	if !cond {
+		t.Helper()
 		if len(val) > 0 {
-			val = append([]interface{}{"assertion failed:"}, val...)
+			val = append([]interface{}{"assertion failed: "}, val...)
 			t.Fatal(val...)
 		} else {
 			t.Fatal("assertion failed")
@@ -40,42 +40,53 @@ func Assert(t testingTB, cond bool, val ...interface{}) {
 
 // Assertf asserts cond is true, otherwise fails the test.
 func Assertf(t testingTB, cond bool, format string, val ...interface{}) {
-	t.Helper()
 	if !cond {
+		t.Helper()
 		t.Fatalf(format, val...)
 	}
 }
 
 // DeepEqual asserts a and b are deep equal, otherwise fails the test.
 func DeepEqual(t testingTB, a, b interface{}) {
-	t.Helper()
 	if !reflect.DeepEqual(a, b) {
+		t.Helper()
 		t.Fatalf("assertion failed: %v != %v", a, b)
 	}
 }
 
 // Panic asserts fn should panic and recover it, otherwise fails the test.
 func Panic(t testingTB, fn func()) {
-	t.Helper()
-	defer func() {
-		if err := recover(); err == nil {
-			t.Fatal("assertion failed: did not panic")
-		}
+	hasPanic := false
+	func() {
+		defer func() {
+			if err := recover(); err != nil {
+				hasPanic = true
+			}
+		}()
+		fn()
 	}()
-	fn()
+	if !hasPanic {
+		t.Helper()
+		t.Fatal("assertion failed: did not panic")
+	}
 }
 
 // PanicAt asserts fn should panic and recover it, otherwise fails the test. The expect function can be provided to do further examination of the error.
 func PanicAt(t testingTB, fn func(), expect func(err interface{}) bool) {
-	t.Helper()
-	defer func() {
-		if err := recover(); err == nil {
-			t.Fatal("assertion failed: did not panic")
-		} else {
-			if expect != nil && !expect(err) {
-				t.Fatal("assertion failed: panic but not expected")
-			}
-		}
+	var err interface{}
+	func() {
+		defer func() {
+			err = recover()
+		}()
+		fn()
 	}()
-	fn()
+	if err == nil {
+		t.Helper()
+		t.Fatal("assertion failed: did not panic")
+		return
+	}
+	if expect != nil && !expect(err) {
+		t.Helper()
+		t.Fatal("assertion failed: panic but not expected")
+	}
 }

internal/test/assert_test.go

@@ -0,0 +1,105 @@
+/*
+ * Copyright 2024 CloudWeGo Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package test
+
+import (
+	"fmt"
+	"testing"
+)
+
+type mockTesting struct {
+	t *testing.T
+
+	expect0 string
+	expect1 string
+
+	helper bool
+}
+
+func (m *mockTesting) Reset() {
+	m.expect0 = ""
+	m.expect1 = ""
+	m.helper = false
+}
+
+func (m *mockTesting) ExpectFatal(args ...interface{}) {
+	m.expect0 = fmt.Sprint(args...)
+}
+
+func (m *mockTesting) ExpectFatalf(format string, args ...interface{}) {
+	m.expect1 = fmt.Sprintf(format, args...)
+}
+
+func (m *mockTesting) Fatal(args ...interface{}) {
+	t := m.t
+	t.Helper()
+	if !m.helper {
+		t.Fatal("need to call Helper before calling Fatal")
+	}
+	if s := fmt.Sprint(args...); s != m.expect0 {
+		t.Fatalf("got %q expect %q", s, m.expect0)
+	}
+}
+
+func (m *mockTesting) Fatalf(format string, args ...interface{}) {
+	t := m.t
+	t.Helper()
+	if !m.helper {
+		t.Fatal("need to call Helper before calling Fatalf")
+	}
+	if s := fmt.Sprintf(format, args...); s != m.expect1 {
+		t.Fatalf("got %q expect %q", s, m.expect1)
+	}
+}
+
+func (m *mockTesting) Helper() { m.helper = true }
+
+func TestAssert(t *testing.T) {
+	m := &mockTesting{t: t}
+
+	m.Reset()
+	m.ExpectFatal("assertion failed")
+	Assert(m, false)
+
+	m.Reset()
+	m.ExpectFatal("assertion failed: hello")
+	Assert(m, false, "hello")
+
+	m.Reset()
+	m.ExpectFatalf("assert: %s", "hello")
+	Assertf(m, false, "assert: %s", "hello")
+
+	m.Reset()
+	m.ExpectFatalf("assertion failed: 1 != 2")
+	DeepEqual(m, 1, 2)
+
+	m.Reset()
+	m.ExpectFatal("")
+	Panic(m, func() { panic("hello") })
+
+	m.Reset()
+	m.ExpectFatal("assertion failed: did not panic")
+	Panic(m, func() {})
+
+	m.Reset()
+	m.ExpectFatal("assertion failed: did not panic")
+	PanicAt(m, func() {}, func(err interface{}) bool { return true })
+
+	m.Reset()
+	m.ExpectFatal("assertion failed: panic but not expected")
+	PanicAt(m, func() { panic("hello") }, func(err interface{}) bool { return false })
+}

pkg/remote/connpool/long_pool_test.go

@@ -567,18 +567,17 @@ func TestLongConnPoolCloseOnIdleTimeout(t *testing.T) {
 	p := newLongPoolForTest(0, 2, 5, idleTime)
 	defer p.Close()
 
-	var closed bool
+	var closed uint32 // use atomic to fix data race issue
 	d := mocksremote.NewMockDialer(ctrl)
 	d.EXPECT().DialTimeout(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(network, address string, timeout time.Duration) (net.Conn, error) {
 		na := utils.NewNetAddr(network, address)
 		conn := mocksnetpoll.NewMockConnection(ctrl)
 		conn.EXPECT().IsActive().Return(true).AnyTimes()
 		conn.EXPECT().RemoteAddr().Return(na).AnyTimes()
 		conn.EXPECT().Close().DoAndReturn(func() error {
-			if closed {
+			if !atomic.CompareAndSwapUint32(&closed, 0, 1) {
 				return errors.New("connection already closed")
 			}
-			closed = true
 			return nil
 		}).AnyTimes()
 		return conn, nil
@@ -589,18 +588,18 @@ func TestLongConnPoolCloseOnIdleTimeout(t *testing.T) {
 
 	c, err := p.Get(context.TODO(), "tcp", addr, opt)
 	test.Assert(t, err == nil)
-	test.Assert(t, !closed)
+	test.Assert(t, atomic.LoadUint32(&closed) == 0)
 
 	err = p.Put(c)
 	test.Assert(t, err == nil)
-	test.Assert(t, !closed)
+	test.Assert(t, atomic.LoadUint32(&closed) == 0)
 
 	time.Sleep(idleTime * 3)
 
 	c2, err := p.Get(context.TODO(), "tcp", addr, opt)
 	test.Assert(t, err == nil)
 	test.Assert(t, c != c2)
-	test.Assert(t, closed) // the first connection should be closed
+	test.Assert(t, atomic.LoadUint32(&closed) == 1) // the first connection should be closed
 }
 
 func TestLongConnPoolCloseOnClean(t *testing.T) {