RI-8219: Integration tests for array read endpoints#6078
Conversation
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
Code Coverage - Backend unit tests
Test suite run success3584 tests passing in 317 suites. Report generated by 🧪jest coverage report action from 6704543 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Code Coverage - Integration Tests
|
![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: 907a9e60d4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The outer `beforeEach(rte.data.truncate)` wipes data between every test, including ACL cases. Only the authorised-user case re-seeded `aclKey` in its `before`; the forbidden-command cases just flipped ACL rules. As a result `checkIfKeyNotExists` returned 404 before Redis ever ran the target array command, so the suite expected 403 but got Not Found on ACL-enabled runs. Reseed `aclKey` via the root `rte.client` (which bypasses ACL) in each deny case's `before`, then flip the ACL rules for the API request. Fixes the same pattern in all seven array integration test files.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7101cf2ae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Code Coverage - Frontend unit tests
Test suite run success7182 tests passing in 816 suites. Report generated by 🧪jest coverage report action from ee9c833 |
ARNEXT on Redis 8.8.0 GA returns 0 for every populated array — its documented summary 'returns the next index ARINSERT would use' is a different operation from what this endpoint exposes (the next safe write index after the highest populated slot). ARLEN already returns max_index + 1 for both dense and sparse arrays, which is exactly the value the endpoint, its response DTO description, and its integration tests promise. Swap the primitive, tighten the response shape (`index` is now non-nullable string), drop the unreachable '(exhausted)' unit case, and remove the now-unused ArNext enum member plus its ioredis builtin registration. Caught by the new oss-st-8 RTE on PR #6078, where every get-next-index Main case was returning index '0' regardless of array shape. Verified against redis:8.8-alpine: ARLEN k → 3 for a dense [0,1,2], 6 for sparse [0,_,_,_,_,5] — matches the test expectations.
The outer `beforeEach(rte.data.truncate)` wipes data between every test, including ACL cases. Only the authorised-user case re-seeded `aclKey` in its `before`; the forbidden-command cases just flipped ACL rules. As a result `checkIfKeyNotExists` returned 404 before Redis ever ran the target array command, so the suite expected 403 but got Not Found on ACL-enabled runs. Reseed `aclKey` via the root `rte.client` (which bypasses ACL) in each deny case's `before`, then flip the ACL rules for the API request. Fixes the same pattern in all seven array integration test files.
pawelangelow
force-pushed
the
be/RI-8219/add-integration-tests
branch
from
10d3ed3 to
8e2f2c9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e2f2c9311
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@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: c3f9cc1560
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@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: 0432a78835
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The outer `beforeEach(rte.data.truncate)` wipes data between every test, including ACL cases. Only the authorised-user case re-seeded `aclKey` in its `before`; the forbidden-command cases just flipped ACL rules. As a result `checkIfKeyNotExists` returned 404 before Redis ever ran the target array command, so the suite expected 403 but got Not Found on ACL-enabled runs. Reseed `aclKey` via the root `rte.client` (which bypasses ACL) in each deny case's `before`, then flip the ACL rules for the API request. Fixes the same pattern in all seven array integration test files.
pawelangelow
force-pushed
the
be/RI-8219/add-integration-tests
branch
from
0432a78 to
6e19140
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72d645ad8d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The outer `beforeEach(rte.data.truncate)` wipes data between every test, including ACL cases. Only the authorised-user case re-seeded `aclKey` in its `before`; the forbidden-command cases just flipped ACL rules. As a result `checkIfKeyNotExists` returned 404 before Redis ever ran the target array command, so the suite expected 403 but got Not Found on ACL-enabled runs. Reseed `aclKey` via the root `rte.client` (which bypasses ACL) in each deny case's `before`, then flip the ACL rules for the API request. Fixes the same pattern in all seven array integration test files.
pawelangelow
force-pushed
the
be/RI-8219/add-integration-tests
branch
from
72d645a to
ee9c833
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee9c8333c1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The outer `beforeEach(rte.data.truncate)` wipes data between every test, including ACL cases. Only the authorised-user case re-seeded `aclKey` in its `before`; the forbidden-command cases just flipped ACL rules. As a result `checkIfKeyNotExists` returned 404 before Redis ever ran the target array command, so the suite expected 403 but got Not Found on ACL-enabled runs. Reseed `aclKey` via the root `rte.client` (which bypasses ACL) in each deny case's `before`, then flip the ACL rules for the API request. Fixes the same pattern in all seven array integration test files.
pawelangelow
force-pushed
the
be/RI-8219/add-integration-tests
branch
from
ee9c833 to
57a399f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 57a399f. Configure here.
The outer `beforeEach(rte.data.truncate)` wipes data between every test, including ACL cases. Only the authorised-user case re-seeded `aclKey` in its `before`; the forbidden-command cases just flipped ACL rules. As a result `checkIfKeyNotExists` returned 404 before Redis ever ran the target array command, so the suite expected 403 but got Not Found on ACL-enabled runs. Reseed `aclKey` via the root `rte.client` (which bypasses ACL) in each deny case's `before`, then flip the ACL rules for the API request. Fixes the same pattern in all seven array integration test files.
pawelangelow
force-pushed
the
be/RI-8219/add-integration-tests
branch
from
57a399f to
6d70bdc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d70bdc488
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Adds integration coverage for the Array read API surface, gated by
`requirements('rte.version>=8.8')` so they only run against RTEs that
ship the Array data type:
- Per-endpoint suites under test/api/array/ for get-count, get-element,
get-elements, get-length, get-next-index, get-range and scan.
- Array branch of the shared /keys/get-info endpoint in
test/api/array/POST-databases-id-keys-get_info-array.test.ts (the
matching branch is removed from the keys suite to avoid duplicate
runs); a forwarding comment is added on the keys side.
- Extra cases for the existing create endpoint:
duplicate-index last-write-wins, 2^64-1 sentinel rejection, and the
symmetric empty-elements guard for sparse mode.
pawelangelow
force-pushed
the
be/RI-8219/add-integration-tests
branch
2 times, most recently
from
6778c49 to
6d8034b
Compare
| ].map(mainCheckFn); | ||
| }); | ||
|
|
||
| // Array coverage for this endpoint lives in |
There was a problem hiding this comment.
nit: not sure if this brings too much value as a comment
2 participants
What
Add integration coverage for the seven Array read endpoints landed
in #6064, plus a Redis 8.8-gated section in
keys/get-infothat exercises thenew
GetArrayKeyInfoResponseoneOfbranch.Eight test files:
array/POST-databases-id-array-get_range.test.ts— ARGETRANGEarray/POST-databases-id-array-scan.test.ts— ARSCANarray/POST-databases-id-array-get_length.test.ts— ARLENarray/POST-databases-id-array-get_count.test.ts— ARCOUNTarray/POST-databases-id-array-get_next_index.test.ts— ARNEXTarray/POST-databases-id-array-get_element.test.ts— ARGETarray/POST-databases-id-array-get_elements.test.ts— ARMGETkeys/POST-databases-id-keys-get_info.test.ts— Array branchWhat the suite pins, beyond what unit specs already cover:
length/count/ scan-index/next-index(incl. a u64 boundary case above
Number.MAX_SAFE_INTEGER);null;RedisStringToBufferTransformernull passthrough onencoding=buffer;limit: nullon scan treated as omitted;@ArrayMinSize(1)and the per-call indexes cap, non-decimal / out-of-u64);WrongType, missing key, missing instance per endpoint;-arget,-arlen,-arcount,-arnext,-armget,-argetrange,-arscan).All assertions go through the existing
deps/validateApiCall/getMainCheckFnharness; every Array describe is gated byrequirements('rte.version>=8.8')so non-preview RTE jobs skip cleanlyrather than fail.
Testing
yarn lint:api— cleanyarn --cwd redisinsight/api type-check— passes against the baselineyarn --cwd redisinsight/api test:api— runs the new suite end-to-endagainst an Array-capable RTE; skips on RTE jobs without Redis 8.8 preview.
Note
Low Risk
Test-only changes with no production API or runtime behavior modified; risk is limited to CI duration on Redis 8.8 array RTE jobs.
Overview
Adds Redis 8.8–gated API integration coverage for the seven array read routes (
get-count,get-element,get-elements,get-length,get-next-index,get-range,scan) using the existingvalidateApiCall/getMainCheckFnharness. Suites assert wire contracts that unit tests do not fully exercise end-to-end: decimal-stringlength/count/indexes (including u64 aboveMAX_SAFE_INTEGER), sparse gap behavior (nullfor empty or past-end slots vs 404 only for missing keys),encoding=bufferwith null passthrough, get-range 1M span caps (including reversed ranges), scanlimitvalidation and huge sparse spans without a span cap, and ARNEXT cursor semantics afterARINSERT/ARSEEK.Each endpoint gets shared validation, wrong-type / missing key / bad instance, and per-command ACL cases. Array create tests gain cases for duplicate sparse indexes (last wins), the 2^64−1 reserved index, and empty sparse
elements.keys/get-infoarray coverage moves totest/api/array/POST-databases-id-keys-get_info-array.test.tsso array-tagged RTE jobs run it without duplicating untagged suites.Reviewed by Cursor Bugbot for commit 6704543. Bugbot is set up for automated code reviews on this repo. Configure here.