Skip to content

Refactor Bloom, JSON and RTS tests#4462

Merged
uglide merged 10 commits into
masterfrom
im/refactor-module-tests-base
Apr 17, 2026
Merged

Refactor Bloom, JSON and RTS tests#4462
uglide merged 10 commits into
masterfrom
im/refactor-module-tests-base

Conversation

@uglide

@uglide uglide commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Note

Low Risk
Low risk because changes are confined to test code and Maven test include patterns, with no production logic changes; main risk is altered CI test selection/execution due to new/removed test classes and tags.

Overview
Refactors module integration testing by replacing legacy RedisModuleCommandsTestBase Bloom/RedisJSON tests with new UnifiedJedisCommandsTestBase-based suites (bloom, json, timeseries) and updating shared JSON test helpers to the non-modules package.

Adds new parameterized RedisClient and Cluster integration tests that reuse these base suites (with cluster gating via @SinceRedisVersion and selective disabling for label-based TimeSeries cluster queries), and updates pom.xml test includes so the new unified module tests are picked up by the build.

Reviewed by Cursor Bugbot for commit 4a93fe3. Bugbot is set up for automated code reviews on this repo. Configure here.

@jit-ci

jit-ci Bot commented Mar 12, 2026

Copy link
Copy Markdown

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Comment thread src/test/java/redis/clients/jedis/commands/unified/json/JsonObjects.java Outdated
@github-actions

github-actions Bot commented Mar 12, 2026

Copy link
Copy Markdown

Test Results

  183 files  +  7    183 suites  +7   11m 36s ⏱️ +34s
8 905 tests +378  8 850 ✅ +534  55 💤  - 156  0 ❌ ±0 
3 359 runs  +126  3 354 ✅ +184   5 💤  -  58  0 ❌ ±0 

Results for commit c8a608d. ± Comparison against base commit 3aa8b7c.

This pull request removes 454 and adds 832 tests. Note that renamed tests count towards both.
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ addExistsMulti
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ addExistsString
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ card
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ info
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ insertExpansion
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ insertNonScaling
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ issue49
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ reserveAlreadyExists
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ reserveBasic
redis.clients.jedis.modules.bloom.BloomTest[1] ‑ reserveEmptyParams
…
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ addExistsMulti
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ addExistsString
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ card
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ info
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ insertExpansion
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ insertNonScaling
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ issue49
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ reserveAlreadyExists
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ reserveBasic
redis.clients.jedis.commands.unified.client.bloom.BloomRedisClientCommandsIT[1] ‑ reserveEmptyParams
…
This pull request removes 12 skipped tests and adds 6 skipped tests. Note that renamed tests count towards both.
redis.clients.jedis.modules.json.RedisJsonV2Test[1] ‑ numIncrByResp3
redis.clients.jedis.modules.json.RedisJsonV2Test[2] ‑ numIncrByResp3
redis.clients.jedis.modules.json.RedisJsonV2Test[3] ‑ numIncrBy
redis.clients.jedis.modules.timeseries.TimeSeriesTest[1] ‑ aggregationTypeSafeValueOf
redis.clients.jedis.modules.timeseries.TimeSeriesTest[1] ‑ countNanAndCountAll
redis.clients.jedis.modules.timeseries.TimeSeriesTest[1] ‑ countNanAndCountAllWithBucketTimestamp
redis.clients.jedis.modules.timeseries.TimeSeriesTest[2] ‑ aggregationTypeSafeValueOf
redis.clients.jedis.modules.timeseries.TimeSeriesTest[2] ‑ countNanAndCountAll
redis.clients.jedis.modules.timeseries.TimeSeriesTest[2] ‑ countNanAndCountAllWithBucketTimestamp
redis.clients.jedis.modules.timeseries.TimeSeriesTest[3] ‑ aggregationTypeSafeValueOf
…
redis.clients.jedis.commands.unified.client.json.RedisJsonV2RedisClientCommandsIT[1] ‑ numIncrByResp3
redis.clients.jedis.commands.unified.client.json.RedisJsonV2RedisClientCommandsIT[2] ‑ numIncrByResp3
redis.clients.jedis.commands.unified.client.json.RedisJsonV2RedisClientCommandsIT[3] ‑ numIncrBy
redis.clients.jedis.commands.unified.cluster.json.RedisJsonV2ClusterCommandsIT[1] ‑ numIncrByResp3
redis.clients.jedis.commands.unified.cluster.json.RedisJsonV2ClusterCommandsIT[2] ‑ numIncrByResp3
redis.clients.jedis.commands.unified.cluster.json.RedisJsonV2ClusterCommandsIT[3] ‑ numIncrBy

♻️ This comment has been updated with latest results.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

@uglide uglide force-pushed the im/refactor-module-tests-base branch from ac3360e to c62c053 Compare March 30, 2026 15:00
@uglide uglide requested review from a-TODO-rov and ggivo March 30, 2026 15:11
@uglide uglide force-pushed the im/refactor-module-tests-base branch from 2543f13 to 8d35a7f Compare April 15, 2026 09:28

@ggivo ggivo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice!
LGTM

Just one question before approving
I see a few tests related to time series are missing compared to the previous TimeSeriesTest.java. Is this intentional, and what it the motivation?

@ggivo ggivo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@uglide uglide merged commit 1f69ead into master Apr 17, 2026
16 of 17 checks passed
@uglide uglide deleted the im/refactor-module-tests-base branch April 17, 2026 13:19
@ggivo ggivo added the testing label May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants