Let the test suite work with "external" redis server

[voxik]

I am trying to package redis-client for Fedora, because it seems to be used by redis. In this attempt, I'd like to run the test suite with "Fedora" distributed version of Redis. Nevertheless, this seems to be pain ATM, because the whole test suite is built around the idea that Redis is downloaded and build.

https://github.com/redis-rb/redis-client/blob/7897ffefb4e8d1f4926bc4ad8d718fac2a4ad02f/test/support/redis_builder.rb#L35

Also, I wonder why toxiproxy, while it is IMHO similar case, is handled completely differently, via install-toxiproxy

[byroot]

the whole test suite is built around the idea that Redis is downloaded and build.

Yes, this is because we need to test against different versions of redis, and on many different platforms, so the easiest is to download and build it. That is the same with redis-rb, so not much change here.

I'm open a PR to make using an already existing redis an option (e.g. via ENV var or something).

I wonder why toxiproxy, while it is IMHO similar case, is handled completely differently, via install-toxiproxy

Because toxiproxy has readily available binaries for each platform, so no point compiling it (and it would require the go toolchain anyway).

[voxik]

the whole test suite is built around the idea that Redis is downloaded and build.

Yes, this is because we need to test against different versions of redis, and on many different platforms, so the easiest is to download and build it.

Right, but the version information is external to the test suite, so the build could also be external.

That is the same with redis-rb, so not much change here.

Just FTR. there were necessary essentially two changes to redis-rb:

1. fake the bin/build script:

# We are using packaged Redis, so provide just dummy Redis build script.
mkdir bin
echo '#!/usr/bin/sh' > bin/build
chmod a+x bin/build

2. Call make with some parameters:

make BINARY=$(which redis-server) REDIS_CLIENT=$(which redis-cli) BUILD_DIR='${TMP}'

To convince redis-client to use the preinstalled Redis, I think I need to do:

$ sed -i '/build_redis/ s/^/#/' test/test_helper.rb
$ sed -i '/def redis_server_bin/,/end/ s/redis_builder.bin_path/"redis-server"/' test/support/servers.rb

Comparing these two approaches, I think that the make approach is more flexible.

I'm open a PR to make using an already existing redis an option (e.g. via ENV var or something).

Thx. Given what I mentioned above, I think I would need to make the build process external to the test suite and therefore:

1. I am not sure it would really be acceptable
2. It would need quite some change
3. Is unfortunately not realistic I'd could afford to spend time to rework this

But I won't be mad if this is not a priority for you and therefore the ticket is closed. I'll keep using the workaround I mentioned above.

I wonder why toxiproxy, while it is IMHO similar case, is handled completely differently, via install-toxiproxy

Because toxiproxy has readily available binaries for each platform, so no point compiling it (and it would require the go toolchain anyway).

I understand and I agree, but again, the toxiproxy is external information to the actual test suite, while the Redis is baked in.

[casperisfine]

Right, but the version information is external to the test suite, so the build could also be external.

I don't follow what you mean here.

Thx. Given what I mentioned above, I think I would need to make the build process external to the test suite and therefore

Again I don't see what you mean here. What I'm suggesting is to have an env var to use whatever redis-server is in the path instead of building it from source.

the toxiproxy is external information to the actual test suite, while the Redis is baked in.

Again, no idea what you mean. We download the redis source, just like we download the toxiproxy binary.

[voxik]

Right, but the version information is external to the test suite, so the build could also be external.

I don't follow what you mean here.

https://github.com/redis-rb/redis-client/blob/8142b12b78216131ef2c145d3008102e64158891/.github/workflows/ci.yml#L53

Here you specify what Redis version is going to be used. Instead, there could have been provided redis executable, while the Redis would be setup in some step prior this one.

Thx. Given what I mentioned above, I think I would need to make the build process external to the test suite and therefore

Again I don't see what you mean here. What I'm suggesting is to have an env var to use whatever redis-server is in the path instead of building it from source.

the toxiproxy is external information to the actual test suite, while the Redis is baked in.

Again, no idea what you mean. We download the redis source, just like we download the toxiproxy binary.

Actually there are 3 external dependencies AFACIT and three distinct methods to obtain them:

1) Redis, downloaded via the redis_builder.rb 2) Toxiproxy downloaded via bin/install-toxiproxy 3) Hiredis downloaded by Rake task

It would be nice if: 1) all these tools used the same method for their setup. 2) something as basic as $PATH environment variable was used to pick e.g. the redis-server executable. I don't think there is any need for something such as redis_server_bin method (at least not in its current form returning redis_builder.bin_path)

[casperisfine]

Actually there are 3 external dependencies AFACIT and three distinct methods to obtain them:

Well. If there was easy to download redis-server binaries I'd use that. I won't be compiling toxiproxy from source just to be consistent with redis.

As for hiredis that's not part of CI, it's just an helper for me to upgrade the vendored hiredis, but that's a manual task.

Anyway, I think I stated the constraints, if you have PRs to make your life easier, I'll happily merge gems as long as they don't make my life harder.