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

Allowing for running tests on a port other than the fixed 6380 #2466

Merged
merged 5 commits into from Mar 6, 2023

Conversation

chayim
Copy link
Contributor

@chayim chayim commented Feb 28, 2023

In testing various versions of redis, it is sometimes necessary to go back and forth between multiple running redis instances. This change makes it possible to specify the instance, purely by the existing port - which includes a README directive.

@chayim
Copy link
Contributor Author

chayim commented Feb 28, 2023

Interesting - the github cli pushed this here, when it explicitly said it would push to the fork 🤷

I'll double check my settings along the way.

@monkey92t
Copy link
Collaborator

monkey92t commented Feb 28, 2023

I developed a tool: https://github.com/monkey92t/grte
It's weak, but helps developers quickly test go-redis.

The dependent redis-server has been installed in docker-image,
Quick run test😁:

monkey@MonkeyiMac redis % grte go test ./...
[go-redis]  🐳 Prepare docker image ==> goredis/grte:v9
[go-redis]  🐳 Use image cache, ID ==> sha256:f4a6149b55e000ed40b5a68041615e7223b52119178d6ec4190f4714b8b88a49
[go-redis]  🐳 Create docker container...
[go-redis]  🐳 ContainerID: c788618272cefc280a86134d2c5315fdd2d9d48e7bf30b948e3efc6ca2076a2e
[go-redis]  🐳 WorkDir: /Users/monkey/Desktop/redis
[go-redis]  🐳 Command: [go test ./...]
ok  	github.com/redis/go-redis/v9	108.379s
ok  	github.com/redis/go-redis/v9/internal	0.016s
ok  	github.com/redis/go-redis/v9/internal/hashtag	0.024s
ok  	github.com/redis/go-redis/v9/internal/hscan	0.022s
ok  	github.com/redis/go-redis/v9/internal/pool	1.568s
ok  	github.com/redis/go-redis/v9/internal/proto	0.010s
?   	github.com/redis/go-redis/v9/internal/rand	[no test files]
?   	github.com/redis/go-redis/v9/internal/util	[no test files]
[go-redis]  ✅  Success!
monkey@MonkeyiMac redis % 

@monkey92t
Copy link
Collaborator

I don't think this is a good way, go-redis test is more complicated, we need to start redis-server, sentinel, cluster, ring server.

@chayim
Copy link
Contributor Author

chayim commented Mar 1, 2023

Depends on your need. I need to test umpteen redises - and dockers don't solve my problems. When I need to go through tonnes and things, and dockers we rely on my redisenv. It'll probably support direct binary eventually.

@chayim
Copy link
Contributor Author

chayim commented Mar 1, 2023

I don't think this is a good way, go-redis test is more complicated, we need to start redis-server, sentinel, cluster, ring server.

Depends what you're testing. You can have a variety of inputs. This allows me to move forward with multis - and run the others after the fact. For my use case - very useful. Again, nothing changes for the user here.

@monkey92t
Copy link
Collaborator

Go-redis needs to stop and start redis-server during test execution, for example:

err = master.Shutdown(ctx).Err()
....
_, err = startRedis(masterPort)

If redis-server is running in other containers, it is difficult for us to start or stop.

@vmihailenco
Copy link
Collaborator

This overrides only a single port out of ~ 10, but no objections to have it 👍

Copy link
Collaborator

@monkey92t monkey92t left a comment

Choose a reason for hiding this comment

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

OK, we can do that for now, but we should find an easier way to test. 😂

@monkey92t monkey92t merged commit cbfe6cd into master Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants