test: add config and rate limit coverage - #685
Merged
rafaelscosta merged 1 commit intoMay 8, 2026
Merged
Conversation
rafaelscosta
requested review from
Pedrovaleriolopez and
oalanicolas
as code owners
May 8, 2026 00:40
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds three Jest test suites: env-interpolator (string/object interpolation, pattern linting, ENV_VAR_PATTERN), merge-utils (isPlainObject, deepMerge, mergeAll), and rate-limit-manager (constructor, error detection, delay logic, retries, metrics, wrapper, singleton). No production code changed. ChangesEnvironment Interpolation Test Suite
Merge Utilities Test Suite
Rate Limit Manager Test Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced May 8, 2026
github-actions
Bot
added
area: agents
rafaelscosta
force-pushed
the
devops/test-config-rate-limit-coverage-20260507
branch
from
May 8, 2026 00:42
4159e93 to
32e0dd7
Compare
Contributor
📊 Coverage ReportCoverage report not available
Generated by PR Automation (Story 6.1) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/core/config/env-interpolator.test.js`:
- Around line 12-17: The test is importing interpolateString,
interpolateEnvVars, lintEnvPatterns, and ENV_VAR_PATTERN via a relative require;
change that require to use the project's absolute import path (e.g., the
package/module root import that maps to .aiox-core/core/config/env-interpolator)
so the line requiring these symbols uses the absolute module name instead of
'../../../.aiox-core/...'; keep the same destructured symbols
(interpolateString, interpolateEnvVars, lintEnvPatterns, ENV_VAR_PATTERN) and
update only the module specifier to the project's approved absolute import.
In `@tests/core/config/merge-utils.test.js`:
- Line 12: The test imports merge utilities using a relative path; update the
require to use the project's absolute module import instead so it follows the
"absolute imports" rule: replace the relative
require('../../../.aiox-core/core/config/merge-utils') in
tests/core/config/merge-utils.test.js with the absolute module import for the
same module and keep the imported symbols (deepMerge, mergeAll, isPlainObject)
unchanged so the tests still reference those functions.
In `@tests/core/execution/rate-limit-manager.test.js`:
- Line 13: Replace the relative require of the RateLimitManager module with an
absolute import: instead of const RateLimitManager =
require('../../../.aiox-core/core/execution/rate-limit-manager'); use the
project-root absolute path import (for example
require('.aiox-core/core/execution/rate-limit-manager') or the project's
configured absolute alias) so the test imports the same module via an absolute
path and satisfies the repository lint rule; adjust to use import syntax if the
codebase prefers ES modules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 719cb66e-eecc-4f53-99e6-1c512b9eec66
📒 Files selected for processing (3)
tests/core/config/env-interpolator.test.jstests/core/config/merge-utils.test.jstests/core/execution/rate-limit-manager.test.js
rafaelscosta
force-pushed
the
devops/test-config-rate-limit-coverage-20260507
branch
from
May 8, 2026 00:49
32e0dd7 to
195fb96
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/core/config/env-interpolator.test.js (1)
14-21: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRelative import — use an absolute module specifier.
The
path.resolve(__dirname, ...)+path.join(...)construction is still a relative import in disguise and violates the project's coding standards.As per coding guidelines, "Use absolute imports instead of relative imports in all code" for
**/*.{js,jsx,ts,tsx}files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/core/config/env-interpolator.test.js` around lines 14 - 21, The test uses a relative-import workaround (path.resolve(__dirname, ...) + path.join(...)) to require the module that exports interpolateString, interpolateEnvVars, lintEnvPatterns, and ENV_VAR_PATTERN; replace that with a single absolute module specifier require call (e.g., require('aiox-core/core/config/env-interpolator') or whatever the project’s canonical absolute package path is) and keep the existing destructured imports so the test imports interpolateString, interpolateEnvVars, lintEnvPatterns, and ENV_VAR_PATTERN via the absolute require instead of constructing a filesystem path.
🧹 Nitpick comments (2)
tests/core/config/env-interpolator.test.js (2)
204-219: ⚡ Quick win
ENV_VAR_PATTERNgloballastIndexcan leak across the preceding test runs.The
interpolateStringandinterpolateEnvVarssuites invoke the production functions that internally consumeENV_VAR_PATTERN(a global regex). If the implementation ever exits aexec()-loop early (e.g., error path),lastIndexcould be left non-zero. While the tests here correctly usenew RegExp(ENV_VAR_PATTERN.source)for their own assertions — so these tests are safe — adding an explicitbeforeEachreset prevents any future direct use ofENV_VAR_PATTERNin this block from being order-dependent.♻️ Proposed safeguard
describe('ENV_VAR_PATTERN', () => { + beforeEach(() => { + ENV_VAR_PATTERN.lastIndex = 0; + }); + it('deve ser uma regex global', () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/core/config/env-interpolator.test.js` around lines 204 - 219, Add a beforeEach in this describe block that resets the global regex state to avoid test-order flakiness: explicitly set ENV_VAR_PATTERN.lastIndex = 0 before each test so that any prior exec() usage by interpolateString or interpolateEnvVars cannot leak a non-zero lastIndex into these specs; reference the ENV_VAR_PATTERN identifier and mention the related functions interpolateString and interpolateEnvVars so maintainers know why the reset is needed.
28-36: ⚡ Quick win
process.envisolation should useafterEach, notafterAll.Both describe blocks restore
process.envonly inafterAll. If a test throws unexpectedly (rather than an assertion failure), the remaining tests in that block still receive a reset env viabeforeEach, but any parallelized or cross-describe side effects accumulate until the entire suite for that block finishes. ReplacingafterAllwithafterEachmakes the cleanup symmetric withbeforeEachand removes any ambiguity.♻️ Proposed fix (applies to both describe blocks at lines 34–36 and 90–92)
- afterAll(() => { + afterEach(() => { process.env = ORIGINAL_ENV; });Also applies to: 84-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/core/config/env-interpolator.test.js` around lines 28 - 36, The test restores process.env in an afterAll but the setup uses beforeEach, so switch the teardown to afterEach to ensure isolation per-test; update the two occurrences of afterAll (() => { process.env = ORIGINAL_ENV; }) to afterEach (() => { process.env = ORIGINAL_ENV; }) in the env-interpolator.test.js file so the beforeEach/afterEach pair correctly reset process.env for each test (apply to both describe blocks that reference ORIGINAL_ENV, beforeEach, and afterAll).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@tests/core/config/env-interpolator.test.js`:
- Around line 14-21: The test uses a relative-import workaround
(path.resolve(__dirname, ...) + path.join(...)) to require the module that
exports interpolateString, interpolateEnvVars, lintEnvPatterns, and
ENV_VAR_PATTERN; replace that with a single absolute module specifier require
call (e.g., require('aiox-core/core/config/env-interpolator') or whatever the
project’s canonical absolute package path is) and keep the existing destructured
imports so the test imports interpolateString, interpolateEnvVars,
lintEnvPatterns, and ENV_VAR_PATTERN via the absolute require instead of
constructing a filesystem path.
---
Nitpick comments:
In `@tests/core/config/env-interpolator.test.js`:
- Around line 204-219: Add a beforeEach in this describe block that resets the
global regex state to avoid test-order flakiness: explicitly set
ENV_VAR_PATTERN.lastIndex = 0 before each test so that any prior exec() usage by
interpolateString or interpolateEnvVars cannot leak a non-zero lastIndex into
these specs; reference the ENV_VAR_PATTERN identifier and mention the related
functions interpolateString and interpolateEnvVars so maintainers know why the
reset is needed.
- Around line 28-36: The test restores process.env in an afterAll but the setup
uses beforeEach, so switch the teardown to afterEach to ensure isolation
per-test; update the two occurrences of afterAll (() => { process.env =
ORIGINAL_ENV; }) to afterEach (() => { process.env = ORIGINAL_ENV; }) in the
env-interpolator.test.js file so the beforeEach/afterEach pair correctly reset
process.env for each test (apply to both describe blocks that reference
ORIGINAL_ENV, beforeEach, and afterAll).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b989d88-e0bd-411d-ad0c-cb5db36ff392
📒 Files selected for processing (3)
tests/core/config/env-interpolator.test.jstests/core/config/merge-utils.test.jstests/core/execution/rate-limit-manager.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/core/execution/rate-limit-manager.test.js
- tests/core/config/merge-utils.test.js
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
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.
Summary
RateLimitManager, including retry behavior, backoff, metrics, event logging, wrapper helper, and singleton manager.env-interpolator, including string interpolation, nested object/array handling, missing-var warnings, lint detection, and regex behavior.merge-utils, including plain object detection, deep merge semantics, append arrays, null deletion, immutability, and mergeAll ordering.Supersedes #598 and #609 with a fresh test-only branch based on current
main.Validation
npm test -- tests/core/config/env-interpolator.test.js tests/core/config/merge-utils.test.js tests/core/execution/rate-limit-manager.test.js --runInBandnpm run lint -- --quiet --ignore-pattern .aiox-core/data/entity-registry.yamlnpm run typechecknpm run validate:manifestgit diff --checkSummary by CodeRabbit