-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: Enable CI for Redis CE 8.0 #3274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ndyakov
commented
Feb 10, 2025
•
edited
Loading
edited
- Extract Benchmark tests
- Speed up CI by using docker images
- Test agains 3 versions of Redis (7.2, 7.4, 8.0)
- Test with latest 2 versions of Go (1.23, 1.24)
- Enable additional CodeQL Analysis
928ecbf
to
a9308b6
Compare
a9308b6
to
c031034
Compare
5f53638
to
9fec123
Compare
a0ee27f
to
1b80811
Compare
1b80811
to
11bdcc8
Compare
0d207b1
to
6818620
Compare
bump go version in CI
171b10d
to
3c6b79e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR introduces CI improvements for Redis CE 8.0 support by speeding up builds using Docker images, updating test configurations, and adding benchmark tests and CodeQL analysis.
- Update CI configurations to support multiple Redis and Go versions
- Add benchmark tests with improved cluster reuse
- Introduce CodeQL analysis workflow and update documentation
Reviewed Changes
File | Description |
---|---|
.github/workflows/codeql-analysis.yml | Added CodeQL analysis workflow |
README.md | Updated supported Redis versions and Go testing info |
.github/workflows/build.yml | Updated build and benchmark job configurations |
docker-compose.yml | Updated service definitions for Redis and sentinel |
commands_test.go | Replaced SkipBeforeRedisMajor with SkipBeforeRedisVersion |
.github/workflows/doctests.yaml | Adjusted Go version used for doctests |
doctests/* | Added FlushDB calls for fresh test environments |
.github/workflows/test-redis-enterprise.yml | Updated Redis version and Go version for enterprise tests |
.github/actions/run-tests/action.yml | Modified test run steps and environment variable usage |
bench_test.go | Reused cluster topology to speed up benchmarks |
cluster_commands.go | Added ClusterMyID command |
acl_commands_test.go | Replaced SkipBeforeRedisMajor with SkipBeforeRedisVersion |
Copilot reviewed 59 out of 59 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
bench_test.go:372
- [nitpick] The removal of the redis process startup and cleanup logic in BenchmarkExecRingSetAddrsCmd could lead to resource management issues if redis processes are instantiated elsewhere. Please verify that cleanup is appropriately handled to prevent any potential resource leaks.
for _, port := range []string{ringShard1Port, ringShard2Port} {
.github/actions/run-tests/action.yml:41
- [nitpick] The explicit sleep delay was removed from the action; ensure that the new approach reliably waits for the Docker Compose environment to be ready to avoid potential race conditions.
sleep 10 # time to settle
Co-authored-by: Copilot <[email protected]>
add information about new test setup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This pull request enables continuous integration for Redis CE 8.0 while optimizing test execution and supporting multiple Redis and Go versions. Key changes include the addition of a CodeQL analysis workflow, updates to build and test workflows (including benchmark improvements and docker-compose adjustments), and modifications in test files to standardize version skipping logic.
Reviewed Changes
File | Description |
---|---|
.github/workflows/codeql-analysis.yml | Added CodeQL analysis workflow for enhanced security scanning. |
README.md | Updated documentation to list supported Redis versions and Go releases. |
CONTRIBUTING.md | Revised contribution instructions to use docker-based test environment. |
.github/workflows/build.yml | Updated build and benchmark jobs to run tests against new Redis and Go versions. |
commands_test.go | Updated skip conditions and test assertions for Redis version handling. |
docker-compose.yml | Modified service definitions to use updated docker images and container names. |
.github/workflows/doctests.yaml | Adjusted Go version and verbosity for doc tests. |
Various doctest files | Inserted FlushDB calls for a fresh database in examples. |
.github/workflows/test-redis-enterprise.yml | Updated test configuration to run with Go 1.24 and Redis 7.4. |
.github/actions/run-tests/action.yml | Revised test actions to map Redis versions and start containers using make commands. |
bench_test.go | Refactored benchmark tests to reuse a shared cluster scenario for consistency. |
cluster_commands.go | Introduced a new ClusterMyID command for Redis clusters. |
acl_commands_test.go | Updated ACL tests to use consistent version skipping logic. |
Copilot reviewed 60 out of 60 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
.github/actions/run-tests/action.yml:24
- Consider escaping the dot in the regex (i.e. use '^\d+.\d+') to ensure a literal dot is matched in version numbers.
redis_version_np=$(echo "$REDIS_VERSION" | grep -oP '^\d+.\d+')
bench_test.go:294
- The introduction of a global 'clusterBench' may lead to shared state issues when running benchmarks in parallel; please ensure the state is isolated or document its thread safety.
if clusterBench == nil {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went through your automation once again and it looks ok!
I didn't go in-depth with the go-code itself, I hope thats ok with, sorry if I miss something important out.