[MOD-15876] tests: run the pytest suite against the enterprise build (RS_TEST_ENTERPRISE) - #10205
Conversation
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8fe07c8be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10205 +/- ##
=========================================
Coverage ? 82.41%
=========================================
Files ? 584
Lines ? 78028
Branches ? 32854
=========================================
Hits ? 64303
Misses ? 13419
Partials ? 306
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd45c1e9c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e0ade9737
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa3259a92e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad3b475933
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b59ed5489f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92c7460dc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42fa362c11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00d56fa9eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b41c4d5765
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 208bdfb897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 679274e0ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
679274e to
ebc48af
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebc48afdc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1e582c68d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
01874e5 to
b0af840
Compare
b0af840 to
8f6cc3a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f6cc3a89e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8f6cc3a to
c89be95
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c89be95549
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| wait_for_bgsave_idle( | ||
| lambda: _orig_execute_command(self, 'INFO', 'persistence'), | ||
| stop_on_error=True, | ||
| ) |
There was a problem hiding this comment.
In RS_TEST_ENTERPRISE runs where an automatic/background save lasts longer than the helper's ~5s polling window, wait_for_bgsave_idle returns a still-truthy value but this caller ignores it and immediately issues SAVE. The three 0.2s retries still fail while rdb_bgsave_in_progress remains set, so any test path using SAVE/dumpAndReload() can still hit the same Background save already in progress race this shim is trying to avoid; keep polling or skip/retry the outer operation when the helper reports that BGSAVE is still active.
Useful? React with 👍 / 👎.
bc9f825 to
042db89
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cc2446604
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if args and str(args[0]).upper() == 'FT.PROFILE': | ||
| raw = _orig_execute_command(self, *args, **kwargs) | ||
| return _strip_shard_id_from_profile(raw) |
There was a problem hiding this comment.
When RS_TEST_ENTERPRISE is set, this leaves FT.PROFILE replies otherwise unchanged after the new standalone topology seeding makes Enterprise searches go through the coordinator. The existing tests/pytests/test_optimizer.py::testOptimizer exact comparisons of Iterators profile and Result processors profile still expect the OSS in-process counters, so the Enterprise standalone suite fails there despite only stripping Shard ID; gate those exact counter assertions or normalize the Enterprise coordinator profile shape.
Useful? React with 👍 / 👎.
2cc2446 to
2e5a84b
Compare
Add Enterprise-aware pytest compatibility gates for running the OSS RediSearch suite against the RSE module on Redis-on-Redis standalone Redis.
Move the class out of the `if RS_TEST_ENTERPRISE:` block and gate only the `Defaults.env_class` activation. Defining the class is a pure, inert definition (never instantiated in OSS: env_class stays unset and DialectEnv's base resolves to Env); only setting env_class is a behaviour-changing side effect, so only that needs the gate. Also removes the now-redundant inline comment (the block comment above already explains the RLTest __new__ hook). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Some CI container images ship a uv too old to validate the committed (revision = 3, git-pinned rltest) lock under `uv sync --locked`. Call install_python.sh from install_python_deps.sh so any workflow using the shared test-deps setup gets a uv >= MIN_UV_VERSION first; it is a no-op when uv already meets the minimum. TODO(MOD-15876): drop this with the rltest git-pin once RLTest publishes the pinned fix to PyPI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Revert the RS_TEST_ENTERPRISE special-case in test_yield_while_bg_indexing_mod4745 back to the OSS bound. The yield code path is identical on enterprise for this RAM index, so relaxing to >=1 only masks coverage. Testing empirically whether the OSS bound actually fails under the RoR environment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_yield_while_bg_indexing_mod4745 counts BG_INDEX yields (one per 100 RM_Scan cursor steps ~= DB-dict buckets). A live bgsave fork defers dict expansion (COW), leaving the dict under-sized and the scan short, so the n//100 bound fails intermittently (observed 5 vs 10 on RoR). Disable periodic save so no fork races the key load; keeps the OSS bound and drops the RS_TEST_ENTERPRISE special-case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RLTest 0.7.28 (with the Defaults.env_class hook, #254) is on PyPI, so replace the temporary git-pin with rltest>=0.7.28,<0.8.0 and regenerate uv.lock (PyPI source, revision=3 preserved). With no git source in the lock, uv 0.5.13 validates it under --locked (verified in-container), so remove the modern-uv guard from install_python_deps.sh (restored to base). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Same fork-GC race the sibling tests hit under RoR: an Enterprise auto-bgsave can start in the gap before RedisModule_Fork, failing EEXIST so the GC cycle collects nothing (the 100 deleted docs survive -> DUMP_TAGIDX not empty, count 150 not 50). forceInvokeGC waits for an in-progress save but had no retry for that residual window; loop until the tag index empties. No-op on OSS (collects on the first pass); edition-agnostic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
a89c22a to
ca0bcd1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca0bcd1434
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| env.expect('FT.PROFILE', 'idx1', 'SEARCH', 'QUERY', '*', "FORMAT", "STRING", 'SCORER', 'TFIDF').equal(exp) | ||
| res = env.cmd('FT.PROFILE', 'idx1', 'SEARCH', 'QUERY', '*', "FORMAT", "STRING", 'SCORER', 'TFIDF') | ||
| env.assertEqual(strip_enterprise_profile_keys(res), exp) |
There was a problem hiding this comment.
When RS_TEST_ENTERPRISE=1 runs this standalone RESP3 profile test, the topology seeding makes FT.PROFILE use the Enterprise coordinator-shaped reply: the cluster expectation just below includes Coordinator dispatch time [ms] in each shard and a non-empty Coordinator object. This assertion only strips Shard ID before comparing against the OSS standalone shape with Coordinator: {}, so the Enterprise run still fails here; gate this exact comparison or normalize the coordinator-only profile fields as well.
Useful? React with 👍 / 👎.
RE does not allow loading extensions (EXTLOAD), so skip the tests that load
the example extension on RS_TEST_ENTERPRISE: test_ext.py::testExt and
test_default_scorer.py::{test_default_scorer_with_extension,
test_default_scorer_startup_validation}. test_default_scorer_behavior (no
extension) still runs. Matches the RoR CI dropping the ext-example build.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| # On enterprise, SEARCH.CLUSTERSET's parser recognizes HASHFUNC/RANGES/NUMSLOTS as keywords | ||
| # before falling back to short-form dispatch, so it reports a "Bad value for ..." parse | ||
| # error instead of the OSS short-form-dispatch error. | ||
| if not RS_TEST_ENTERPRISE: |
There was a problem hiding this comment.
Why is this not the case on RS_TEST_ENTERPRISE?
There was a problem hiding this comment.
because the current Redis pin doesn't support it. Advancing Redis pin to latest rl_8.x is deferred from this PR to MOD-17151
Default enterprise to None so a test can be skipped on enterprise (True), on OSS (False), or not gated (None) — mirroring the cluster predicate. Fix the one call site that passed a computed bool so its non-search-ext-load configs stay None rather than silently becoming skip-on-OSS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 3-arg forms reach short-form dispatch only when RedisModule_GetClusterNodeSlotRanges is present (Redis #14953). Our pinned enterprise Redis predates that patch, so they hit the long-form parser instead — not because enterprise "recognizes keywords first". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9253c20f37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if RS_TEST_ENTERPRISE: | ||
| Defaults.env_class = RediSearchEnterpriseStandaloneEnv |
There was a problem hiding this comment.
When RS_TEST_ENTERPRISE=1, the pinned RLTest 0.7.28 runner creates test environments through Defaults.env_factory; it has no Env.__new__ redirection and never reads Defaults.env_class. This assignment therefore only creates an unused attribute, while ordinary function tests—and the @skip wrapper's direct Env(...) construction—continue using RLTest.Env, so _seed_topology() is never called and coordinator-gated commands fail with ERRCLUSTER Uninitialized cluster state. Install the subclass through Defaults.env_factory and ensure direct/manual environment construction uses it as well.
Useful? React with 👍 / 👎.
| # defers dict expansion while a persistence fork is live (copy-on-write), which | ||
| # would leave the dict under-sized and the scan short. Disable periodic RDB save | ||
| # so no bgsave fork races the key load and the count stays deterministic. | ||
| run_command_on_all_shards(env, 'CONFIG', 'SET', 'save', '') |
There was a problem hiding this comment.
When the suite runs in the supported EXT=1 existing-environment mode, RLTest reuses the server and only flushes data between tests, so this CONFIG SET persists after the test finishes. It leaves automatic RDB persistence disabled for every subsequent test—and for the externally managed server after the suite—rather than merely preventing a BGSAVE during this load. Capture the current save value and restore it in a finally block, or skip this case for existing environments.
Useful? React with 👍 / 👎.
4 participants
Summary
Adds
RS_TEST_ENTERPRISEcompatibility gates so the OSS pytest suite runs unmodified against the Enterpriseredisearch.soon RoR standalone.Key changes, all no-ops on OSS (
RS_TEST_ENTERPRISEunset):FT.CONFIGGET/SET to_FT.CONFIG, and normalizeFT.PROFILEreplies (strip Enterprise-onlyShard ID).NumShardsinit untilSEARCH.CLUSTERSET.WORKERS/MIN_OPERATION_WORKERSmax stable at 16 despite higher internal worker capacity).GC_FORCEINVOKE).Validation: full OSS pytest suite passes against the RSE module on standalone
rl_8.6.Release notes: not required.
Release Notes