Skip to content

Conversation

@weim0000
Copy link
Contributor

@weim0000 weim0000 commented May 7, 2025

image
image

I want to add some test case for commandzmscore and commandzrandmember, but I cloud not find any folder or file to test commandXXX.

after fix:

image
image

@aleksraiden aleksraiden changed the title fix(zset):zrandmember and zmscore cmd reply protocal err fix(zset): zrandmember and zmscore cmd reply protocal err May 7, 2025
@PragmaTwice PragmaTwice changed the title fix(zset): zrandmember and zmscore cmd reply protocal err fix(zset): wrong RESP reply in ZRANDMEMBER and ZMSCORE command May 7, 2025
@git-hulk
Copy link
Member

git-hulk commented May 7, 2025

I want to add some test case for commandzmscore and commandzrandmember, but I cloud not find any folder or file to test commandXXX.

@weim0000 You could add test cases in zset_test.go

@PragmaTwice
Copy link
Member

sorry, [zset_test.go] test redis_zset.cc, but these bugs is in cmd_zset.cc, there is no xx_test.go for cmd_xx.cc

In golang test cases, we start a kvrocks instance and connect it via go-redis client library. So that these RESP replies can be tested well. No need to add a test file for cmd_xx.cc.

createZset(rdb, ctx, "zset", z)

// ZRANDMEMBER zset
str := rdb.Do(ctx, "ZRANDMEMBER", "zset").Val()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can use rdb.ZRandMember.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems not a major issue, maybe we can merge it first.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it can't use rdb.ZRandMember, because this function have a count param,
ZRandMember(ctx context.Context, key string, count int);
and this bug must not input count param;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thank you!

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 7, 2025

@PragmaTwice PragmaTwice merged commit 496f9d4 into apache:unstable May 7, 2025
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants