Use system property to point to a custom redis server#3
Use system property to point to a custom redis server#3martin-g wants to merge 1 commit intocodemonstur:masterfrom martin-g:use-system-property-to-custom-redis-server
Conversation
| static ExecutableProvider newRedis2_8_19Provider() { | ||
| return newRedis2_8_19Map()::get; | ||
| return osArchitecture -> { | ||
| if (CUSTOM_PATH != null) { |
There was a problem hiding this comment.
IMO if a custom path is specified then it should be preffered.
But there is a unit test that breaks due to this new behavior.
Up to you to decide whether to drop the test or to use the CUSTOM_PATH as a fallback.
There was a problem hiding this comment.
I made the whole thing configurable. So now it is up to the user to decide what kind of ExecutableProvider they want.
| /** | ||
| * A system property that allows to configure a custom path for redis-server | ||
| */ | ||
| String CUSTOM_PATH = System.getProperty("com.github.codemonstur.embedded-redis"); |
There was a problem hiding this comment.
I don't like this approach. I rewrote the ExecutableProvider a little to make it easier to configure something custom. So if someone wants to use a property they can do so:
final File executable = new File(System.getProperty("pick.something"));
final ExecutableProvider provider = () -> executable;| map.put(WINDOWS_x86_64, "/redis-server-2.8.19.exe"); | ||
| map.put(UNIX_x86, "/redis-server-2.8.19-32"); | ||
| map.put(UNIX_x86_64, "/redis-server-2.8.19"); | ||
| map.put(UNIX_AARCH64, "/redis-server-2.8.19-linux-aarch64"); |
There was a problem hiding this comment.
I actually already replaced your binary with the one I generated with QEMU. It probably doesn't work but until that is confirmed I can just leave it in.
There was a problem hiding this comment.
It seems to work just fine for embedded-redis' unit tests and all unit tests of Apache Dubbo!
| * Temporary disabled until deciding what should be the behavior of | ||
| * {@link ExecutableProvider#newRedis2_8_19Provider()} | ||
| */ | ||
| @Ignore |
There was a problem hiding this comment.
In my changes this test still works fine.
Yeah this is bad. The problem actually isn't that the server is already running, but that both the service and the tests use the same port. The proper solution is to rewrite the tests to use random unused ports instead. I haven't gotten there because the tests are still so broken and its just an annoying pile of work. |
Related to #2 (comment)
An annoying issue is that
apt install redis-serverstarts the server and one needs to stop it (systemctl stop redis-server.service) with before being able to run the tests. So one needs to be careful to stop the service in the CI.