Java: add IT for pubsub by Yury-Fridlyand · Pull Request #400 · Bit-Quill/valkey-glide

Yury-Fridlyand · 2024-06-28T02:51:07Z

Kittens
Add tests from Python: adds PUB/SUB testing valkey-io/valkey-glide#1643
Add more tests for the god of tests
Resolve some TODOs

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

java/integTest/src/test/java/glide/standalone/PubSubTests.java

acarbonetto · 2024-06-28T19:14:43Z

java/integTest/src/test/java/glide/standalone/PubSubTests.java

+            if (client instanceof RedisClusterClient) {
+                ((RedisClusterClient) client).customCommand(new String[] {"unsubscribe"}).get();
+                ((RedisClusterClient) client).customCommand(new String[] {"punsubscribe"}).get();
+                ((RedisClusterClient) client).customCommand(new String[] {"sunsubscribe"}).get();


is this necessary, or do we clean it up when we destroy the client?

Maybe it is not needed, but I want to explicitly unsubscribe in case if a client isn't GC'ed and remains active/connected.

The client gets close()'d right after these commands run, so I wouldn't think there's a risk of it not getting cleaned up here (not necessarily GC'd).

java/integTest/src/test/java/glide/standalone/PubSubTests.java

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

…va/dev_yuryf_pubsub_tests Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

java/integTest/src/test/java/glide/standalone/PubSubTests.java

jduo · 2024-06-29T15:37:09Z

java/integTest/src/test/java/glide/standalone/PubSubTests.java

+    @ParameterizedTest(name = "use callback = {0}")
+    @ValueSource(booleans = {true, false})
+    public void sharded_pubsub(boolean useCallback) {
+        assumeTrue(REDIS_VERSION.isGreaterThanOrEqualTo("7.0.0"), "This feature added in redis 7");


Redis -> Valkey?

This goes to the test report only. Valkey <7 doesn't exist. Still need to update that?

jduo · 2024-06-29T15:56:01Z

java/integTest/src/test/java/glide/standalone/PubSubTests.java

+    @SuppressWarnings("unchecked")
+    @ParameterizedTest(name = "standalone = {0}, use callback = {1}")
+    @MethodSource("getTwoBoolPermutations")
+    public void exact_happy_path(boolean standalone, boolean useCallback) {


Can we add comments similar to what was in the Python port?

Added references to python tests

java/integTest/src/test/java/glide/standalone/PubSubTests.java

jduo · 2024-06-29T16:02:10Z

java/integTest/src/test/java/glide/standalone/PubSubTests.java

+        var sender = (RedisClusterClient) createClient(false);
+        clients.addAll(List.of(listener, sender));
+
+        assertEquals(1L, sender.spublish(channel, message).get());


Let's add a publishAndWait() helper since this is used alot. Could have it take in an expected result from publish and a Message... varargs so you can include the assertion in it.

jduo · 2024-06-29T16:06:39Z

java/integTest/src/test/java/glide/standalone/PubSubTests.java

+                        standalone ? PubSubChannelMode.PATTERN : PubSubClusterChannelMode.PATTERN,
+                        Set.of(pattern));
+
+        for (var i = 0; i < numChannels; i++) {


This nested loop also shows up in alot of tests. Refactor to GenerateChannelsAndMessages() that takes in output channel and message lists.

java/integTest/src/test/java/glide/standalone/PubSubTests.java

acarbonetto · 2024-06-29T16:52:36Z

java/integTest/src/test/java/glide/PubSubTests.java

+    }
+
+    // TODO add following tests from https://github.com/aws/glide-for-redis/pull/1643
+    //  test_pubsub_exact_happy_path_coexistence


still todo?

Those are blocked by getPubSubMessage, which is not implemented

acarbonetto · 2024-06-29T16:53:01Z

java/integTest/src/test/java/glide/PubSubTests.java

+    private void skipTestsOnMac() {
+        assumeFalse(
+                System.getProperty("os.name").toLowerCase().contains("mac"),
+                "PubSub doesn't work on mac OS");


shocking content

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

java/integTest/src/test/java/glide/PubSubTests.java

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

* Java: Add client configuration for subscribing to channels. (#381) * Add client configuartion for subscribing to channels. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * CLIPPY I HATE YOU Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Get and store callback. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix tests. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Rework configuration and add docs. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Config rework. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * docs Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * More TODOs for the god of TODOs. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add `PUBLISH` and `SPUBLISH` commands. (#391) * Add `PUBLISH` and `SPUBLISH` commands. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix the test. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Java client: receive pubsub messages (#385) * Add client configuartion for subscribing to channels. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * CLIPPY I HATE YOU Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Get and store callback. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix tests. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Rework configuration and add docs. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Config rework. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * docs Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Receive pushes (subscibed messages). Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * I HATE YOU SPOTLESS Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Rename a class. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Java: add IT for pubsub (#400) * Add some tests. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Test fixes. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add more tests. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Experiment Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add more tests. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * I HATE YOU SPOTLESS Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Uncomment test timeout. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Typo fix. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Update function signature. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Yury-Fridlyand added 3 commits June 27, 2024 10:09

Add some tests.

1c072c7

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Test fixes.

c78eb27

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Add more tests.

4550c2c

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Yury-Fridlyand added the java label Jun 28, 2024

jduo reviewed Jun 28, 2024

View reviewed changes

java/integTest/src/test/java/glide/standalone/PubSubTests.java Outdated Show resolved Hide resolved

jduo reviewed Jun 28, 2024

View reviewed changes

java/integTest/src/test/java/glide/standalone/PubSubTests.java Outdated Show resolved Hide resolved

acarbonetto reviewed Jun 28, 2024

View reviewed changes

Yury-Fridlyand added 9 commits June 28, 2024 16:45

Address PR comments.

3f10b0a

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Experiment

35cec92

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Add more tests.

e6814c2

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Typo fix.

5329771

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Merge remote-tracking branch 'origin/java/integ_yuryf_pubsub' into ja…

e4c681c

…va/dev_yuryf_pubsub_tests Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Typo fix.

6bef574

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

I HATE YOU SPOTLESS

1f22d0f

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Typo fix.

d0acd46

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Uncomment test timeout.

e5402f5

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

jduo reviewed Jun 29, 2024

View reviewed changes

java/integTest/src/test/java/glide/standalone/PubSubTests.java Outdated Show resolved Hide resolved

jduo reviewed Jun 29, 2024

View reviewed changes

java/integTest/src/test/java/glide/standalone/PubSubTests.java Outdated Show resolved Hide resolved

jduo reviewed Jun 29, 2024

View reviewed changes

java/integTest/src/test/java/glide/standalone/PubSubTests.java Outdated Show resolved Hide resolved

jduo reviewed Jun 29, 2024

View reviewed changes

java/integTest/src/test/java/glide/standalone/PubSubTests.java Outdated Show resolved Hide resolved

acarbonetto reviewed Jun 29, 2024

View reviewed changes

Address PR comments.

afe56dd

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

acarbonetto approved these changes Jun 29, 2024

View reviewed changes

java/integTest/src/test/java/glide/PubSubTests.java Outdated Show resolved Hide resolved

java/integTest/src/test/java/glide/PubSubTests.java Outdated Show resolved Hide resolved

java/integTest/src/test/java/glide/PubSubTests.java Outdated Show resolved Hide resolved

Typo fix.

391dd32

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Yury-Fridlyand added 3 commits June 29, 2024 12:18

Typo fix.

385aff2

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Typo fix.

ec77170

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Typo fix.

edb920f

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Yury-Fridlyand merged commit 5939973 into java/integ_yuryf_pubsub Jun 29, 2024

Yury-Fridlyand deleted the java/dev_yuryf_pubsub_tests branch June 29, 2024 20:02

Conversation

Yury-Fridlyand commented Jun 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jduo Jun 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Yury-Fridlyand commented Jun 28, 2024 •

edited

Loading

jduo Jun 29, 2024 •

edited

Loading