Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--bugfix="assert.NotNil not work" #1179

Closed
wants to merge 2 commits into from
Closed

Conversation

ElvisWai
Copy link
Contributor

#1178
修复 assert的 NotNil 不生效。(参数判断有误)

In functions that don't work due to argument judgement
@ElvisWai ElvisWai requested review from a team as code owners August 26, 2024 10:59
@CLAassistant
Copy link

CLAassistant commented Aug 26, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ElvisWai
❌ liziwei


liziwei seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ElvisWai
Copy link
Contributor Author

ElvisWai commented Aug 27, 2024

Describe the bug

使用 assert 的 NotNil 断言时不生效

Replay
测试代码:

import  "github.com/cloudwego/hertz/pkg/common/test/assert"
func TestAssertNotNil(t *testing.T) {
	var test interface{}
	assert.NotNil(t, test)
	fmt.Println("test 值不为空")
}

image

Hertz version:

version: 0.9.2

bugfix

func NotNil(t testing.TB, data interface{}) {
	t.Helper()
	if data != nil {
		return
	}

	if reflect.ValueOf(data).IsNil() {
		t.Fatalf("assertion failed, unexpected: %v, expected: not nil", data)
	}
}

@welkeyever
Copy link
Member

有一些 ut 没过,可以瞅瞅

@ElvisWai
Copy link
Contributor Author

ElvisWai commented Aug 28, 2024

@welkeyever
🐶 这是原本的 ut 写错了
Nil 和 NotNil 原本期望是达到什么效果?

@welkeyever
Copy link
Member

@welkeyever 🐶 这是原本的 ut 写错了 Nil 和 NotNil 原本期望是达到什么效果?

嗯,看起来是的,可以一并修复下对应的 ut

@ElvisWai
Copy link
Contributor Author

@welkeyever
有问题的ut改完了,但提交的账号不一致。。。这能解决吗

@welkeyever
Copy link
Member

可能需要统一下,可以本地 squash 成一个 commit

@ElvisWai
Copy link
Contributor Author

我重新提交PR

@ElvisWai ElvisWai closed this Aug 28, 2024
@ElvisWai ElvisWai mentioned this pull request Aug 28, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants