Conversation
Co-authored-by: hawkeyexl <5209367+hawkeyexl@users.noreply.github.com>
- Created new detectTests.ts with browser-compatible parseContent function - Extracted pure parsing logic (no fs, path dependencies) - Added helper functions: parseXmlAttributes, parseObject, replaceNumericVariables - Updated common exports to include detectTests functionality - Successfully compiles with TypeScript strict mode Co-authored-by: hawkeyexl <5209367+hawkeyexl@users.noreply.github.com>
- Made crypto.randomUUID browser-compatible with fallbacks - Added detectTests() function as main entry point for test detection - Removed duplicate interface declarations - All 441 tests passing - Module compiles successfully with TypeScript strict mode Co-authored-by: hawkeyexl <5209367+hawkeyexl@users.noreply.github.com>
- Add unit tests to verify the export of functions from the CommonJS and ESM modules. - Implement tests to ensure the `detectTests` function works correctly with both import styles. - Confirm that the results from `detectTests` are identical when using CJS and ESM. - Check for type definitions related to `detectTests` in the generated TypeScript declaration files. - Remove obsolete test files: `temp-element-timeout-test.json` and `temp-network-timeout-test.json`.
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a browser-safe test-detection module and its types; surfaces detectTests in public exports; reworks build to emit CJS + ESM wrappers; replaces Node-only UUID usage with a browser fallback; removes file I/O and path-resolution modules and their tests; updates package exports, tests, and tooling settings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgba(220,220,255,0.5)
actor Caller
end
rect rgba(200,255,200,0.5)
participant DT as detectTests
participant PC as parseContent
participant LG as Logger
participant VA as Validator
end
Caller->>DT: detectTests({ content, filePath, fileType, config })
DT->>PC: parseContent({ content, filePath, fileType, config })
PC->>LG: log(config, "info"/"warn", message)
PC->>PC: extract inline/markup statements → assemble tests & steps
alt schema conversion needed
PC->>VA: transform & validate steps/tests
VA-->>PC: validation result
end
PC-->>DT: DetectedTest[]
DT-->>Caller: DetectedTest[]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
hawkeyexl
changed the title
detectTests function works correctly with both import styles.
- Confirm that the results from detectTests are identical when using CJS and ESM.
- Check for type definitions related to detectTests in the generated TypeScript declaration files.
- Remove obsolete test files: temp-element-timeout-test.json and temp-network-timeout-test.json.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/detectTests.ts`:
- Around line 366-406: The "testStart" and other switch case bodies leak
block-scoped declarations (e.g., const parsedTest, const parsedStep, let step,
const validation) because their case bodies are not wrapped; wrap each case body
in its own block (add { ... } around the code inside cases like "testStart" and
the step/validation cases around lines where parsedStep/step/validation are
declared) so those const/let bindings are scoped to the case, keep existing
breaks inside the blocks, and ensure any case-local variables (parsedTest,
parsedStep, step, validation) are only used inside their respective blocks.
- Around line 476-506: The code assumes step.httpRequest.request exists when
normalizing headers and body; update the guards to check for the presence of
request before accessing its properties (e.g., ensure step.httpRequest.request
is truthy or use optional chaining) so the blocks that reference
step.httpRequest.request.headers and step.httpRequest.request.body only run when
request is defined; apply this check in both the headers parsing branch and the
JSON body parsing branch (locations referencing step.httpRequest.request.headers
and step.httpRequest.request.body) to prevent TypeError when httpRequest has no
request property.
- Around line 358-419: The ignore flag is toggled in the
"ignoreStart"/"ignoreEnd" cases but never used, so either remove it or enforce
it: update the switch handling for "testStart", "step", and "detectedStep" (the
cases that create/append steps and tests) to check if (ignore) { break; } at the
top of each case to skip processing while inside an ignore block; alternatively,
if ignore blocks are meant to be removed, delete the ignore variable and the
"ignoreStart"/"ignoreEnd" cases (keep testEnd reset logic if still needed).
Ensure references are to the existing ignore variable and the switch cases named
"testStart", "step", and "detectedStep".
- Around line 591-607: The log function treats "warning" and "warn" as different
precedence because normalization happens after computing configLevelIndex and
messageLevelIndex; in function log normalize both the incoming level and
config.logLevel (map "warning" to "warn") before computing logLevels.indexOf so
they are synonyms, remove duplicate "warning" from the logLevels array (keep
only "warn"), and then use the normalizedLevel for the console call
(console[normalizedLevel as 'error'|'warn'|'info'|'debug']) so threshold checks
and output are consistent.
In `@src/validate.ts`:
- Line 35: The dynamic defaults factory for UUID uses a zero-arity function
which violates ajv-keywords' requirement; update
dynamicDefaultsDef.DEFAULTS.uuid to be a one-argument function that returns a
generated UUID (e.g., change the factory from () => getRandomUUID to (_?: any)
=> getRandomUUID()) so the function.length > 0 contract is satisfied and the
UUID is actually produced when invoked; alternatively, if you intended lazy
evaluation keep the parameterized form (args?: any) => () =>
getRandomUUID()—apply the change around dynamicDefaultsDef.DEFAULTS.uuid and
ensure getRandomUUID is called (getRandomUUID()) rather than referenced.
In `@test/detectTests.test.js`:
- Around line 1192-1194: The closing brace that ends the
describe("parseContent", ...) block is in the wrong spot: move the standalone
`});` that currently closes `describe("parseContent", ...)` (just before the
`it("should handle markup regex with no capture groups...")` test) so that it
instead comes after the last `it(...)` that tests parseContent (after line
1371), ensuring all six orphaned `it()` blocks remain inside the
describe("parseContent", ...) block; verify the describe closing brace now
precedes the outer `describe("detectTests module", ...)` continuation and run
tests to confirm scope is correct.
🧹 Nitpick comments (3)
src/validate.ts (1)
12-23: Duplicated UUID fallback logic withsrc/detectTests.ts.
getRandomUUID()here andgenerateUUID()insrc/detectTests.ts(lines 13–22) are identical implementations. Consider extracting a shared helper (e.g., in autils.tsmodule) to avoid maintaining the same code in two places.scripts/createEsmWrapper.js (1)
13-13: ESM wrapper exports must stay in sync withsrc/index.ts.This is a manual, fragile synchronization point. If a future export is added to
src/index.tsbut not mirrored here, ESM consumers silently lose access to it. Consider generating this list programmatically (e.g., by introspectingcjsModulekeys) or at least adding a comment cross-referencingsrc/index.ts.src/detectTests.ts (1)
317-318: Regex constructed from configuration input — low ReDoS risk but worth noting.
statementRegexandpatterncome fromfileTypeconfiguration (developer-controlled, not end-user input). The risk is low, but a malicious or accidental pathological regex in config could cause hangs. Consider documenting thatfileTyperegex patterns should be tested for safety, or add a timeout/complexity check if this will be exposed to untrusted config.Also applies to: 330-331
src/validate.ts
Outdated
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ajv-keywords dynamicDefaults contract factory function signature
💡 Result:
dynamicDefaults keyword contract (schema shape)
dynamicDefaults must be used at the object-schema level and its value is an object mapping property names to either:
- a string (the dynamic-default function id), or
- an object
{ func: string, args: object }whereargsare passed during schema compilation. [1]
Type shape (informal):
type DynamicDefaultsKeyword =
Record<string, string | { func: string; args?: Record<string, any> }>Notes/constraints:
- It only works when Ajv is configured with
useDefaults. [1] - Properties mentioned in
dynamicDefaultsshould not also be inrequiredin the same schema (validation would fail) because these defaults are applied after validation. [1]
Custom dynamic default “factory” function contract (Ajv Keywords)
To add your own function id, you register it in the exported DEFAULTS table:
- Ajv v8 + ajv-keywords v5:
require("ajv-keywords/dist/definitions/dynamicDefaults").DEFAULTS[2] - Ajv v6 + ajv-keywords v3:
require("ajv-keywords").get("dynamicDefaults").definition.DEFAULTS[3]
Signature/behavior contract (as documented):
- A custom default function is selected by id (
func/ string form), and Ajv differentiates functions by arity (function.length). [1] - If your default accepts parameters from schema (
args), implement it as:(args: any) => /* something usable as default */(documentation examples return a 0-arg function that produces the value). [3]
- If your default does not expect
args, it should still have at least one non-optional argument sofunction.length > 0(per ajv-keywords docs) to avoid ambiguity. [1]
So the practical “factory” shapes you see in docs are:
// parameterized (called at compile time with args; returns generator)
type ParamDefaultFactory = (args: Record<string, any>) => () => any
// non-parameterized (docs discuss arity rules; examples vary by major version)
type NonParamDefault = (...atLeastOneArg: any[]) => anySources: ajv-keywords keyword docs and examples for dynamicDefaults and DEFAULTS registration. [1] [2] [3]
🏁 Script executed:
cat -n src/validate.ts | head -50Repository: doc-detective/common
Length of output: 2030
Fix the dynamicDefaults factory function arity to match the ajv-keywords contract.
The ajv-keywords library requires non-parameterized custom defaults to have function.length > 0 (at least one argument). The current () => getRandomUUID has arity 0 and violates this requirement. Change to (_?: any) => getRandomUUID() (or use the parameterized form (args?: any) => () => getRandomUUID() if lazy evaluation is intended).
🤖 Prompt for AI Agents
In `@src/validate.ts` at line 35, The dynamic defaults factory for UUID uses a
zero-arity function which violates ajv-keywords' requirement; update
dynamicDefaultsDef.DEFAULTS.uuid to be a one-argument function that returns a
generated UUID (e.g., change the factory from () => getRandomUUID to (_?: any)
=> getRandomUUID()) so the function.length > 0 contract is satisfied and the
UUID is actually produced when invoked; alternatively, if you intended lazy
evaluation keep the parameterized form (args?: any) => () =>
getRandomUUID()—apply the change around dynamicDefaultsDef.DEFAULTS.uuid and
ensure getRandomUUID is called (getRandomUUID()) rather than referenced.
There was a problem hiding this comment.
Pull request overview
This PR expands the package’s public surface to include a new detectTests module (and related helpers), ensures the ESM wrapper re-exports the same API as CommonJS, and adds integration/type-definition tests around those exports.
Changes:
- Add new browser-compatible
detectTestsimplementation and export it (plus related helpers/types) from the package entrypoint. - Update the ESM wrapper generator to re-export the new APIs from the CJS build.
- Add/extend integration tests to validate CJS/ESM exports and verify generated
.d.tsfiles fordetectTests.
Reviewed changes
Copilot reviewed 6 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration.test.js | Adds CJS/ESM export assertions and cross-module parity checks for detectTests and helpers. |
| test/detectTests.test.js | Adds extensive unit coverage for detectTests helpers (parseContent, parseObject, etc.). |
| src/validate.ts | Reworks UUID default generation to be browser-compatible via dynamicDefaults. |
| src/index.ts | Exports detectTests and related helper functions/types from the main entrypoint. |
| src/detectTests.ts | Introduces the browser-compatible detectTests implementation and helper utilities. |
| scripts/createEsmWrapper.js | Updates ESM wrapper to re-export detectTests and helper exports from the CJS module. |
| package-lock.json | Lockfile metadata adjustments (peer flags removed on some entries). |
| dist/validate.js | Compiled output for src/validate.ts reflecting UUID/dynamicDefaults changes. |
| dist/validate.js.map | Sourcemap updated for validate.js. |
| dist/validate.d.ts.map | Declaration map updated for validate.d.ts. |
| dist/index.mjs | ESM wrapper updated to re-export new APIs. |
| dist/index.js | Compiled entrypoint updated to export detectTests and helpers. |
| dist/index.js.map | Sourcemap updated for index.js. |
| dist/index.d.ts | Type exports updated to include detectTests and helpers. |
| dist/index.d.ts.map | Declaration map updated for index.d.ts. |
| dist/detectTests.js | Compiled output for new src/detectTests.ts. |
| dist/detectTests.js.map | Sourcemap added for detectTests.js. |
| dist/detectTests.d.ts | Type declarations added for detectTests. |
| dist/detectTests.d.ts.map | Declaration map added for detectTests.d.ts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it("should export detectTests function via require", function () { | ||
| const { detectTests } = require("../dist/index.js"); | ||
| expect(typeof detectTests).to.equal("function"); | ||
| }); |
There was a problem hiding this comment.
These newly added assertions are inside a file that is wrapped in an async IIFE (awaiting import("chai")). With Mocha running in CommonJS mode (npm test is just mocha), Mocha does not await the IIFE promise while loading the test file, so the describe/it blocks (including these new ones) may never be registered/executed. Switching to synchronous Chai loading and removing the async wrapper will make these integration tests actually run.
src/validate.ts
Outdated
There was a problem hiding this comment.
dynamicDefaultsDef.DEFAULTS.uuid is being set to a function that returns getRandomUUID (the function), not a UUID string. Ajv's dynamicDefaults will then populate stepId/testId with a function value instead of a string, breaking schema validation/consumers. Set it to a generator that returns a string (e.g., assign getRandomUUID directly or return getRandomUUID() depending on the ajv-keywords API).
| dynamicDefaultsDef.DEFAULTS.uuid = () => getRandomUUID; | |
| dynamicDefaultsDef.DEFAULTS.uuid = getRandomUUID; |
| case "detectedStep": | ||
| test = findTest({ tests, testId }); | ||
| if (typeof test.detectSteps !== "undefined" && !test.detectSteps) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
ignoreStart/ignoreEnd toggle the ignore flag, but the detectedStep handler never checks ignore, so ignore blocks currently have no effect. If ignore markers are meant to suppress markup-detected steps, add an if (ignore) break/return guard in the detectedStep case (and/or skip collecting markup matches while ignore is true).
| }); | ||
| if (!valid.valid) { | ||
| console.warn(`Step ${JSON.stringify(step)} isn't a valid step. Skipping.`); | ||
| return; |
There was a problem hiding this comment.
parseContent emits warnings via console.warn regardless of config.logLevel. Since this module introduces a log() helper that respects logLevel, these warnings should go through that helper (or be gated) to avoid noisy output in library/browser consumers and to make logLevel: "silent" effective.
|
|
||
| // Normalize 'warning' to 'warn' | ||
| const normalizedLevel = level === "warning" ? "warn" : level; | ||
|
|
There was a problem hiding this comment.
log() can throw if called with level: "silent" because normalizedLevel becomes "silent" and console["silent"] is undefined. Consider treating message-level "silent" as a no-op (or removing it from the allowed message levels) so the function is safe for any level in logLevels.
| // Treat message-level 'silent' as a no-op to avoid calling an undefined console method | |
| if (normalizedLevel === "silent") return; |
test/detectTests.test.js
Outdated
There was a problem hiding this comment.
Wrapping test registration in an async IIFE and awaiting import("chai") means Mocha (running in CommonJS mode via npm test: mocha) will not wait for the async setup while loading the file. The describe/it blocks can be registered too late, causing tests to be skipped or nondeterministic. Prefer synchronous Chai loading (e.g., require("chai")) and remove the async IIFE (or convert the test file to ESM in a way Mocha will await).
- Add block scope to case statements in parseContent for better readability. - Update logging function to normalize 'warning' to 'warn' for consistency. - Replace console warnings with the log function for better logging control. - Modify uuid dynamic default to accept arguments in validate.ts. - Clean up test files by removing unnecessary async IIFE wrappers.
- Delete `files.js`, `index.js`, `resolvePaths.js`, `schemas/index.js`, and `validate.js` as they are no longer needed.
…dencies - Remove the `readFile` function and its associated tests from the codebase. - Delete the `resolvePaths` function and its related tests. - Update `package.json` to remove `axios` from dependencies and add `@apidevtools/json-schema-ref-parser` to devDependencies. - Adjust exports in `index.ts` to remove deprecated functions. - Update tests to reflect changes in exports and remove tests for deleted functions.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/integration.test.js`:
- Around line 157-178: The test shares a single mutable testInput between two
calls to detectTests (in the "should produce identical detectTests results from
CJS and ESM" test), which can hide mutations; fix it by cloning testInput before
each invocation (e.g., use JSON.parse(JSON.stringify(testInput)) or
structuredClone) so call sites use separate objects for
cjsModule.detectTests(...) and esmModule.detectTests(...), referencing the
existing variables cjsModule, esmModule, and detectTests to locate where to
apply the clones.
🧹 Nitpick comments (3)
test/detectTests.test.js (3)
11-33: Test-scoped constants are defined outside thedescribeblock.
markdownFileTypeandminimalFileType(lines 11–32) are indented as if they belong inside thedescribe("detectTests module", ...)block starting at line 34, but they're actually at module scope. This works due to closure, but the misleading indentation could confuse contributors.Move the constant declarations inside the
describeblock, or de-indent them to make the module-level scope explicit.
168-179: Duplicate test scenario for escaped JSON.The test at line 168 ("should parse escaped JSON (double-encoded)") and the test at line 174 ("should fallback to simple unescaping for escaped JSON") use the identical input string
'{\\"key\\": \\"value\\"}'. The second test has a weaker assertion (.to.not.be.null) and doesn't exercise a different code path from the first. Consider removing the duplicate or crafting an input that actually requires the simple-unescape fallback (where double-parse fails butreplace(/\\"/g, '"')succeeds).
296-396: Console monkey-patching is fragile; consider using Sinon stubs.Sinon is already a devDependency. Using
sinon.stub(console, 'info')would be cleaner and automatically restore viasinon.restore()inafterEach, avoiding manual save/restore of each console method.
- Add tests to normalize logLevel configurations in the log function - Implement tests to ensure messages are not logged when logLevel is set to silent - Add tests to skip detected steps inside ignore blocks in markdown content - Use structuredClone for test input in integration tests to prevent mutation
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/detectTests.ts`:
- Around line 248-270: The recursive branch in replaceNumericVariables assigns
the result of replaceNumericVariables(stringOrObject[key], values) back to
stringOrObject[key] which can be null for unresolvable nested properties; change
this so that you capture the recursive result into a variable and if that result
is null delete stringOrObject[key] (matching the string branch behavior)
otherwise assign the non-null result back—this ensures nested properties are
removed rather than set to null.
In `@test/detectTests.test.js`:
- Around line 1153-1167: The test "should skip inline steps inside ignore block"
creates a test with all inline steps placed inside an ignore block which results
in zero steps and the test being filtered out by validation; update the test
content used by parseContent so at least one step exists outside the ignore
block (e.g., add a seed step comment before the "<!-- test ignore -->" block or
include a non-ignored step in the test metadata) so result[0].steps is non-empty
and the assertion after parseContent remains valid.
- Around line 1137-1151: The test fails because tests with zero steps are
removed by validation; update the "should skip detected steps inside ignore
block" test so the seeded test is not empty: add a valid step outside the ignore
block in the content string and adjust the assertion accordingly (use
parseContent result from parseContent and expect result to have one test and
result[0].steps length to reflect the seeded step, or alternatively change the
assertion to expect length 0 if you intentionally want empty tests dropped).
Locate the test by its description and the content variable in the file and
modify the content and the expect(...) checks to match the chosen fix.
🧹 Nitpick comments (5)
src/validate.ts (1)
11-22: Duplicate UUID helper — extract to a shared module.
getRandomUUID()insrc/validate.tsis identical togenerateUUID()insrc/detectTests.ts(lines 13–22). Consider extracting a singleuuid.tsutility and importing it from both files.src/detectTests.ts (2)
317-318: Regex from configuration input — be aware of ReDoS risk.Both
statementRegex(line 317) and markuppattern(line 330) are compiled fromfileTypeconfiguration intonew RegExp(...). If a consumer passes untrusted or adversarial regex patterns, this could lead to catastrophic backtracking. Since this module is designed as a library API, consider documenting thatfileTypepatterns must be trusted input, or adding a timeout/complexity guard if untrusted sources are expected.
13-22: Duplicate UUID helper — same asgetRandomUUIDinsrc/validate.ts.This is a copy of the identical function in
validate.ts. Both should share a single implementation (see comment onsrc/validate.tslines 11–22).test/detectTests.test.js (1)
11-32: Test fixtures declared outsidedescribeblock scope.
markdownFileTypeandminimalFileTypeare declared at module level (outside thedescribeblock starting at line 34). While this works, it's a minor style inconsistency — consider moving them inside thedescribeblock for encapsulation, or marking themconstat module level is fine too since they're alreadyconstand not mutated.test/integration.test.js (1)
231-274: Package.json is re-read in every test.Each test under "Package exports field verification" calls
fs.readFileSync+JSON.parseindependently. Abeforehook or sharedlet packageJsonwould reduce redundancy, though this is a minor nit for test code.
| if (typeof stringOrObject === "object") { | ||
| Object.keys(stringOrObject).forEach((key) => { | ||
| if (typeof stringOrObject[key] === "object") { | ||
| stringOrObject[key] = replaceNumericVariables(stringOrObject[key], values); | ||
| } else if (typeof stringOrObject[key] === "string") { | ||
| const matches = stringOrObject[key].match(/\$[0-9]+/g); | ||
| if (matches) { | ||
| const allExist = matches.every((variable: string) => { | ||
| const index = variable.substring(1); | ||
| return Object.hasOwn(values, index) && typeof values[index] !== "undefined"; | ||
| }); | ||
| if (!allExist) { | ||
| delete stringOrObject[key]; | ||
| } else { | ||
| stringOrObject[key] = stringOrObject[key].replace(/\$[0-9]+/g, (variable: string) => { | ||
| const index = variable.substring(1); | ||
| return values[index]; | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Recursive replaceNumericVariables can return null for nested properties, leaving null values in the object.
When a nested object property contains unresolvable variables (line 251), the recursive call returns null, which is then assigned back: stringOrObject[key] = null. This differs from the string-property behavior (line 260) which deletes the key. Consider aligning the two behaviors.
Proposed fix
if (typeof stringOrObject[key] === "object") {
- stringOrObject[key] = replaceNumericVariables(stringOrObject[key], values);
+ const replaced = replaceNumericVariables(stringOrObject[key], values);
+ if (replaced === null) {
+ delete stringOrObject[key];
+ } else {
+ stringOrObject[key] = replaced;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof stringOrObject === "object") { | |
| Object.keys(stringOrObject).forEach((key) => { | |
| if (typeof stringOrObject[key] === "object") { | |
| stringOrObject[key] = replaceNumericVariables(stringOrObject[key], values); | |
| } else if (typeof stringOrObject[key] === "string") { | |
| const matches = stringOrObject[key].match(/\$[0-9]+/g); | |
| if (matches) { | |
| const allExist = matches.every((variable: string) => { | |
| const index = variable.substring(1); | |
| return Object.hasOwn(values, index) && typeof values[index] !== "undefined"; | |
| }); | |
| if (!allExist) { | |
| delete stringOrObject[key]; | |
| } else { | |
| stringOrObject[key] = stringOrObject[key].replace(/\$[0-9]+/g, (variable: string) => { | |
| const index = variable.substring(1); | |
| return values[index]; | |
| }); | |
| } | |
| } | |
| } | |
| }); | |
| } | |
| if (typeof stringOrObject === "object") { | |
| Object.keys(stringOrObject).forEach((key) => { | |
| if (typeof stringOrObject[key] === "object") { | |
| const replaced = replaceNumericVariables(stringOrObject[key], values); | |
| if (replaced === null) { | |
| delete stringOrObject[key]; | |
| } else { | |
| stringOrObject[key] = replaced; | |
| } | |
| } else if (typeof stringOrObject[key] === "string") { | |
| const matches = stringOrObject[key].match(/\$[0-9]+/g); | |
| if (matches) { | |
| const allExist = matches.every((variable: string) => { | |
| const index = variable.substring(1); | |
| return Object.hasOwn(values, index) && typeof values[index] !== "undefined"; | |
| }); | |
| if (!allExist) { | |
| delete stringOrObject[key]; | |
| } else { | |
| stringOrObject[key] = stringOrObject[key].replace(/\$[0-9]+/g, (variable: string) => { | |
| const index = variable.substring(1); | |
| return values[index]; | |
| }); | |
| } | |
| } | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@src/detectTests.ts` around lines 248 - 270, The recursive branch in
replaceNumericVariables assigns the result of
replaceNumericVariables(stringOrObject[key], values) back to stringOrObject[key]
which can be null for unresolvable nested properties; change this so that you
capture the recursive result into a variable and if that result is null delete
stringOrObject[key] (matching the string branch behavior) otherwise assign the
non-null result back—this ensures nested properties are removed rather than set
to null.
Summary by CodeRabbit
New Features
Chores
Tests