Fix CI failure issues #321
Open
+19,862
−19,750
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix 7 failing tests in test_self_contained_coordinator_memtier.py by: - Adding messages to arch-specific streams instead of base stream - Fixing consumer group creation parameters (arch and id) - Updating assertions to check arch-specific streams This aligns tests with the arch-specific stream routing implemented in the coordinator, which reads from streams like: - oss:api:gh/redis/redis/builds:amd64 (for amd64) - oss:api:gh/redis/redis/builds:arm64 (for arm64) Fixes: - test_self_contained_coordinator_dockerhub_preload - test_self_contained_coordinator_dockerhub - test_self_contained_coordinator_dockerhub_iothreads - test_self_contained_coordinator_dockerhub_valkey - test_dockerhub_via_cli - test_dockerhub_via_cli_airgap - test_self_contained_coordinator_duplicated_ts
Apply black formatting to test_self_contained_coordinator_memtier.py to comply with CI code style checks.
Apply black formatting to runner.py for FLUSHALL changes from PR redis#320 to comply with CI code style checks.
Improve metric validation to skip operation-specific metrics when those operations are not in the tested-commands list. For example, a SET-only test (--ratio 1:0) will now skip validation of Gets.Ops/sec metric, which would legitimately be 0. This fixes CI test failures where SET-only tests were failing validation because Gets.Ops/sec was 0 (below the 10 QPS threshold). The validation now: 1. Checks benchmark_config for 'tested-commands' list 2. Skips metrics for operation types (gets, sets, hgets, etc.) that are not in the tested-commands list 3. Still validates metrics for operations that are actually being tested
Add git_version parameter when calling generate_benchmark_stream_request
in tests. This ensures version information is propagated through the stream
to the coordinator, allowing by.version keys to be created in Redis.
Without this, the tests expect by.version keys to exist but they were never
created because artifact_version was None in the data export logic.
Fixes test assertions that check for:
- ci.benchmarks.redis/.../by.version/{version}/benchmark_end/.../memory_maxmemory
Tests fixed:
- test_self_contained_coordinator_dockerhub_preload
- test_self_contained_coordinator_dockerhub
- test_self_contained_coordinator_dockerhub_iothreads
- test_self_contained_coordinator_dockerhub_valkey
- test_self_contained_coordinator_duplicated_ts
- Add regex-based version extraction from image names like 'redis:7.4.0' or 'valkey/valkey:7.2.6-bookworm' - Pass extracted git_version to generate_benchmark_stream_request() - Enables by.version Redis TimeSeries keys to be created for CLI-triggered tests - Fixes test_dockerhub_via_cli and test_dockerhub_via_cli_airgap assertion failures
Apply black formatting to comply with CI code style checks.
The export_redis_metrics function creates keys with the format:
{prefix}/{test}/{by_variant}/benchmark_end/{running_platform}/{setup}/{metric}
But tests were expecting keys without running_platform:
{prefix}/{test}/{by_variant}/benchmark_end/{setup}/{metric}
This fixes all 6 failing tests by adding running_platform to the expected key format.
Also removes debug print statements that helped diagnose the issue.
This commit fixes stats validation errors:
1. Add JSON module commands to commands.json:
- JSON.GET: Return value at path in JSON serialized form
- JSON.SET: Sets or updates JSON value at a path
2. Add 'json' group to groups.json with JSON commands
3. Fix test-suite metadata issues:
- memtier_benchmark-playbook-rate-limiting-lua-100k-sessions.yml:
Remove 'bitmap' from tested-groups (only scripting is tested via EVAL)
- memtier_benchmark-playbook-realtime-analytics-membership.yml:
Add 'sunion' to tested-commands (SUNION command was used but not listed)
- memtier_benchmark-playbook-realtime-analytics-membership-pipeline-10.yml:
Add 'sunion' to tested-commands
These changes resolve all stats validation errors in CI.
…r_memtier Pull Request: Fix test_self_contained_coordinator_memtier tests and stats validation This PR fixes multiple issues causing CI failures in the self-contained coordinator tests and stats validation. Issues Fixed: Architecture-specific stream routing - Tests were adding metrics to base streams but coordinators were reading from arch-specific streams (:amd64/:arm64 suffixes), causing NOGROUP errors in 7 tests Missing running_platform in key paths - Tests failed to find by.version keys because the key format was missing the running_platform component in 6 tests Metric validation false positives - Validation incorrectly failed when untested operation types had zero metrics (e.g., SET-only tests reporting 0 Gets/sec) Version extraction from Docker images - CLI couldn't extract git_version from Docker image tags like redis:7.4.0, breaking version-based key lookups Missing RedisJSON module support - Stats validation failed because JSON.GET and JSON.SET commands were not defined in commands.json, and the json group was missing from groups.json Test metadata mismatches - Three test YAMLs had incorrect metadata: rate-limiting test declared bitmap group but only uses scripting, and membership tests were missing sunion from tested-commands Changes: Fixed 13 tests in test_self_contained_coordinator_memtier.py Enhanced metric validation to skip untested operations Added RedisJSON commands and group definitions Corrected test suite metadata in 3 YAML files
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issues Fixed:
Architecture-specific stream routing - Tests were adding metrics to base streams but coordinators were reading from arch-specific streams (:amd64/:arm64 suffixes), causing NOGROUP errors in 7 tests
Missing running_platform in key paths - Tests failed to find by.version keys because the key format was missing the running_platform component in 6 tests
Metric validation false positives - Validation incorrectly failed when untested operation types had zero metrics (e.g., SET-only tests reporting 0 Gets/sec)
Version extraction from Docker images - CLI couldn't extract git_version from Docker image tags like redis:7.4.0, breaking version-based key lookups
Missing RedisJSON module support - Stats validation failed because JSON.GET and JSON.SET commands were not defined in commands.json, and the json group was missing from groups.json
Test metadata mismatches - Three test YAMLs had incorrect metadata: rate-limiting test declared bitmap group but only uses scripting, and membership tests were missing sunion from tested-commands