Skip to content

Conversation

uglide
Copy link

@uglide uglide commented Sep 19, 2025

  • Standalone only
  • redis_commands workload only

@uglide uglide requested review from Copilot and ggivo September 19, 2025 14:39
@uglide uglide marked this pull request as draft September 19, 2025 14:39
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a basic Jedis test application to the existing Lettuce test framework, supporting standalone mode with redis_commands workload. The implementation mirrors the structure and behavior of the existing Lettuce test application while providing Jedis-specific functionality.

  • Creates a new Jedis test application with standalone Redis connection support
  • Implements redis_commands workload for Jedis using the same pattern as Lettuce
  • Configures build profiles to support both Lettuce and Jedis applications

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/main/java/redis/clients/jedis/test/JedisTestApplication.java Main application class for Jedis test app with Spring Boot configuration
src/main/java/redis/clients/jedis/test/JedisWorkloadRunner.java Workload runner component managing Jedis test execution
src/main/java/redis/clients/jedis/test/StandaloneJedisWorkloadRunner.java Standalone mode implementation for Jedis workload execution
src/main/java/redis/clients/jedis/test/workloads/JedisBaseWorkload.java Base workload class providing common functionality for Jedis tests
src/main/java/redis/clients/jedis/test/workloads/JedisRedisCommandsWorkload.java Redis commands workload implementation for Jedis
src/main/java/redis/clients/jedis/test/SchedulerConfig.java Spring configuration for metrics reporter scheduler
src/main/java/redis/clients/jedis/test/JedisRunner.java Interface defining runner contract
src/main/java/io/lettuce/test/workloads/BaseWorkload.java Changes field visibility to protected for Jedis inheritance
src/main/java/io/lettuce/test/metrics/MetricsReporter.java Makes methods public for Jedis access
src/main/java/io/lettuce/test/config/WorkloadRunnerConfig.java Adds configurable defaults property source
src/main/java/io/lettuce/test/config/TestRunProperties.java Updates app name to use spring.application.name
src/main/resources/application.properties Makes log file names dynamic using application name
src/main/resources/logback-spring.xml Updates log file paths to use application name
pom.xml Adds Jedis dependencies and build profiles for multiple applications

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +53 to +54
}
return null;
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

The catch block swallows the exception and returns null instead of rethrowing it. This could mask errors and lead to unexpected null values. Consider rethrowing the exception after recording metrics.

Suggested change
}
return null;
throw ex;
}

Copilot uses AI. Check for mistakes.

Copy link
Contributor

@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

I have concerns about storing both tools in the same repository and attempting to share existing classes from the lettuce test app, particularly around Runner/workloads/metrics components.

Most of the logic is tightly bound to the specific library we are testing. For example, the existing test workloads operate on lettuce-specific types like StatefulRedisConnection and RedisClient. We will need to duplicate these for Jedis implementation.

Similarly, ClientOptionsConfig is specific to the Lettuce application and won't be reusable for Jedis.

While there is some reusable content like utilities for generating test values and metrics reporting, it would probably be easier to either:

  1. Copy the source code and modify it for Jedis in a different repository, or
  2. Consider creating a multi-module Maven project with separate applications for Lettuce/Jedis and some common shared tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants