Replace "redis" with "valkey" test code by Shivshankar-Reddy · Pull Request #287 · valkey-io/valkey

Shivshankar-Reddy · 2024-04-10T21:28:47Z

Occurrences of "redis" in TCL test suites and helpers, such as TCL client used in tests, are replaced with "valkey".

Occurrences of uppercase "Redis" are not changed in this PR.

No files are renamed in this PR.

Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>

Shivshankar-Reddy · 2024-04-12T13:37:08Z

@enjoy-binbin @zuiderkwast I have changed most of redis to valkey in this PR, I tried changing meaningfull names but it gave lot of trouble and test case failure so I went with replacing redis with "valkey". If you have some time could you please have a look at this?

zuiderkwast · 2024-04-12T14:53:42Z


-    const char *scheme = "redis://";
-    const char *tlsscheme = "rediss://";
+    const char *scheme = "valkey://";
+    const char *tlsscheme = "valkeys://";
    const char *curr = uri;


This is not test code.

This is a breaking change. It is #198. See the discussions in #199.

We're not doing that. Besides, it is interfering with #199.

We already have had several PRs doing the same changes and we're reviewing them over and over.

I got your point, But I had benchmark: tls connecting using URI with authentication set,get and benchmark: connecting using URI with authentication set,get test failed. that was the reason I changed, No problem we can wait until #199 (#198) gets merged.

I'm not sure #198 will get merged at all.

Either way, I think we should test the redis:// protocol to make sure we support it. We want valkey-cli 8.0 to be compatible with valkey-server 7.2.x and with redis oss.

Ohh.. Do we have any macro or check for backport compatibilty defined, I can go ahead update the commit in this pr or first I will commit them in different PR as they are going to be breaking changes and later we can focus on this PR changes. WDYT?

I think just keep redis:// and rediss:// in this PR. Only update internal names in the test.

We are sure we want to support it and still test it.

We are not yet sure if we want to add another scheme. If we do, we want to test both.

thank you for the details, sure I will change internally in test and commit again!

Completed the changes and updated tests as well to pass now, when scheme is added we can change those test together. Thanks!

Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>

enjoy-binbin · 2024-04-18T02:22:50Z

full ci: https://github.com/valkey-io/valkey/actions/runs/8731463488

Shivshankar-Reddy · 2024-04-18T13:31:16Z

full ci: https://github.com/valkey-io/valkey/actions/runs/8731463488

Thank you @enjoy-binbin , I will look into it and fix.

zuiderkwast

Very good. I will merge it before there are new merge conflicts. :)

Just to be clear what this PR is doing.

I can see only lowercase "redis" is replaced by "valkey".

It does not change "Redis" (in the output of ./runtest --help) but we can do that in another PR.

It also does not rename any files. We can do that in another PR too. (Rename redis.tcl to valkey.tcl.)

Shivshankar-Reddy · 2024-04-18T18:58:20Z

full ci: https://github.com/valkey-io/valkey/actions/runs/8731463488

Hi @enjoy-binbin , The failures happen on Mac OS workflows and I dont have Mac book so I have tried replicate the same tests on Ubuntu machine for multiple times and I could not replicate them.

From the logs we could see below tests failed.

We can read back the value we set before
Create a 5 nodes cluster

We may need help fom someone who has Mac and test this cases and see whether its related to CI or fails on manual run as well..

Occurrences of "redis" in TCL test suites and helpers, such as TCL client used in tests, are replaced with "valkey". Occurrences of uppercase "Redis" are not changed in this PR. No files are renamed in this PR. --------- Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>

Renamed below procedures and variables (missed in #287) as follows. redis_cluster -> valkey_cluster redis1 -> valkey1 redis2 -> valkey2 get_redis_dir -> get_valkey_dir test_redis_cli_rdb_dump -> test_valkey_cli_rdb_dump test_redis_cli_repl -> test_valkey_cli_repl redis-cli -> valkey-cli redis_reset_state -> valkey_reset_state redisHandle -> valkeyHandle redis_safe_read -> valkey_safe_read redis_safe_gets -> valkey_safe_gets redis_write -> valkey_write redis_read_reply -> valkey_read_reply redis_readable -> valkey_readable redis_readnl -> valkey_readnl redis_writenl -> valkey_writenl redis_read_map -> valkey_read_map redis_read_line -> valkey_read_line redis_read_null -> valkey_read_null redis_read_bool -> valkey_read_bool redis_read_double -> valkey_read_double redis_read_verbatim_str -> valkey_read_verbatim_str redis_call_callback -> valkey_call_callback --------- Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>

Renamed below procedures and variables (missed in valkey-io#287) as follows. redis_cluster -> valkey_cluster redis1 -> valkey1 redis2 -> valkey2 get_redis_dir -> get_valkey_dir test_redis_cli_rdb_dump -> test_valkey_cli_rdb_dump test_redis_cli_repl -> test_valkey_cli_repl redis-cli -> valkey-cli redis_reset_state -> valkey_reset_state redisHandle -> valkeyHandle redis_safe_read -> valkey_safe_read redis_safe_gets -> valkey_safe_gets redis_write -> valkey_write redis_read_reply -> valkey_read_reply redis_readable -> valkey_readable redis_readnl -> valkey_readnl redis_writenl -> valkey_writenl redis_read_map -> valkey_read_map redis_read_line -> valkey_read_line redis_read_null -> valkey_read_null redis_read_bool -> valkey_read_bool redis_read_double -> valkey_read_double redis_read_verbatim_str -> valkey_read_verbatim_str redis_call_callback -> valkey_call_callback --------- Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com> Signed-off-by: Sher Sun <sher.sun@huawei.com>

Squashed 'deps/libvalkey/' changes from 45c2ed15c..cb5ff91aa cb5ff91aa Release 0.5.0 cd3d5d8ff Fix potential heap-buffer-overflow in cluster error reply parsing (#305) 32c7c5d09 Refactor error handling to use valkeySetErrorFromErrno and valkeyClearError (#303) e4b548b32 Remove support for external dict while still supporting external sds (#302) 61b27c453 Try all addresses from DNS before failing to connect (#300) 8aff94fab RDMA: Fix lost EPOLLIN/POLLIN events b9285b5fc Optimize read buffer compaction and reduce copying (#294) 0fa809877 Additional overflow protection for MAP/ATTR b26c56e87 Fix gcc warnings (#287) git-subtree-dir: deps/libvalkey git-subtree-split: cb5ff91aa5816807f0a549bd2b36e611253dfeb6 Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>

rename test proc to valkey

3e30944

Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>

enjoy-binbin mentioned this pull request Apr 11, 2024

Rename redis.tcl to valkey.tcl #283

Merged

Shivshankar-Reddy added 3 commits April 11, 2024 20:48

more changes regarding rename in test dir

a183835

Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>

adjust time to test pass

ab37c0c

Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>

adjust time to test pass

dbbb173

Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>

zuiderkwast reviewed Apr 12, 2024

View reviewed changes

zuiderkwast mentioned this pull request Apr 14, 2024

Replace all "redis" keywords to "valkey" in tcl test files #155

Closed

remove scheme changes

766465f

Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>

zuiderkwast approved these changes Apr 18, 2024

View reviewed changes

zuiderkwast changed the title ~~Rename test procedures to valkey~~ Replace "redis" with "valkey" test code Apr 18, 2024

zuiderkwast merged commit 34413e0 into valkey-io:unstable Apr 18, 2024

Shivshankar-Reddy deleted the rebrand-test-proc branch April 18, 2024 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace "redis" with "valkey" test code#287

Replace "redis" with "valkey" test code#287
zuiderkwast merged 5 commits into
valkey-io:unstablefrom
Shivshankar-Reddy:rebrand-test-proc

Shivshankar-Reddy commented Apr 10, 2024 •

edited by zuiderkwast

Loading

Uh oh!

Shivshankar-Reddy commented Apr 12, 2024

Uh oh!

zuiderkwast Apr 12, 2024

Uh oh!

Shivshankar-Reddy Apr 12, 2024

Uh oh!

zuiderkwast Apr 13, 2024

Uh oh!

Shivshankar-Reddy Apr 15, 2024

Uh oh!

zuiderkwast Apr 17, 2024

Uh oh!

Shivshankar-Reddy Apr 17, 2024

Uh oh!

Shivshankar-Reddy Apr 17, 2024

Uh oh!

enjoy-binbin commented Apr 18, 2024

Uh oh!

Shivshankar-Reddy commented Apr 18, 2024

Uh oh!

zuiderkwast left a comment

Uh oh!

Shivshankar-Reddy commented Apr 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Shivshankar-Reddy commented Apr 10, 2024 • edited by zuiderkwast Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Shivshankar-Reddy commented Apr 12, 2024

Uh oh!

zuiderkwast Apr 12, 2024

Choose a reason for hiding this comment

Uh oh!

Shivshankar-Reddy Apr 12, 2024

Choose a reason for hiding this comment

Uh oh!

zuiderkwast Apr 13, 2024

Choose a reason for hiding this comment

Uh oh!

Shivshankar-Reddy Apr 15, 2024

Choose a reason for hiding this comment

Uh oh!

zuiderkwast Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

Shivshankar-Reddy Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

Shivshankar-Reddy Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

enjoy-binbin commented Apr 18, 2024

Uh oh!

Shivshankar-Reddy commented Apr 18, 2024

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Shivshankar-Reddy commented Apr 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Shivshankar-Reddy commented Apr 10, 2024 •

edited by zuiderkwast

Loading