test migration#66
Merged
Merged
Conversation
Closed
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR migrates tests from Node’s test/assert runner to Jest, expands test discovery and lint/format globs, and standardizes test import paths across core and core-ui packages. ChangesJest Configuration and Per-Package Test Setup
API and MCP Test Framework Migration to Jest
Core and Core-UI Test Assertion and Import Path Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core-ui/src/tests/utils/options-json-panel.test.js (1)
97-109:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
toBe('undefined')asserts string-coercion ofundefinedrather than a guarded default.When
setFromOptionsreceives an object with nooptionsproperty,options.options.prettyPrintDelimiterisundefined. SettinginputElement.value = undefinedin the DOM silently coerces that to the string"undefined". The assertion on Line 108 codifies this unguarded path instead of the intended default.The implementation should null-guard before writing to the input (e.g.,
value ?? ''), and the test should assert the sane default:🐛 Suggested assertion correction (after guarding the implementation)
- expect(parent.querySelector("input[name='custom-pretty-delimiter']").value).toBe('undefined'); + expect(parent.querySelector("input[name='custom-pretty-delimiter']").value).toBe('');🤖 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 `@packages/core-ui/src/tests/utils/options-json-panel.test.js` around lines 97 - 109, The test reveals that JsonOptionsPanel.setFromOptions writes undefined into the DOM causing the input[name='custom-pretty-delimiter'] to become the string "undefined"; fix setFromOptions in JsonOptionsPanel (the code path that handles options.options.prettyPrintDelimiter) to null-guard when assigning to DOM inputs (e.g., use value ?? '' or similar) so undefined becomes an empty string, and update the test assertion to expect '' for input[name='custom-pretty-delimiter'] instead of 'undefined'.
🧹 Nitpick comments (5)
packages/core/src/tests/data_generation/unit/rulesParser.test.js (1)
4-4: ⚡ Quick winUse consistent ES module syntax.
The file mixes CommonJS
require()with ES moduleimportstatements. For consistency and to avoid potential module interoperability issues, consider converting this to an ES module import.♻️ Proposed refactor to use ES module syntax
-const RandExp = require('randexp'); +import RandExp from 'randexp';🤖 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 `@packages/core/src/tests/data_generation/unit/rulesParser.test.js` at line 4, The test mixes CommonJS and ESM by using const RandExp = require('randexp');—replace the CommonJS require with an ES module import for the RandExp identifier (i.e., import RandExp from 'randexp') so the file consistently uses ESM; ensure the RandExp import is used wherever the RandExp symbol appears in the tests and that the project/test runner is configured to support ESM if needed.packages/core-ui/src/tests/utils/options-csv-delimited-controls.test.js (1)
10-21: 💤 Low valueConsider cleaning up global variables in afterEach.
The test sets
global.windowandglobal.documentinbeforeEachbut doesn't clean them up inafterEach. While Jest's test isolation may handle this, explicitly resetting these globals can prevent potential cross-test contamination.♻️ Optional cleanup pattern
afterEach(() => { dom.window.close(); + delete global.window; + delete global.document; });🤖 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 `@packages/core-ui/src/tests/utils/options-csv-delimited-controls.test.js` around lines 10 - 21, The test sets global.window and global.document in beforeEach (creating JSDOM and instantiating CsvDelimitedOptions and calling panel.addToGui) but afterEach only closes dom.window; update afterEach to also delete or reset global.window and global.document (and optionally parent/panel) to avoid cross-test contamination — locate the beforeEach/afterEach block and modify the afterEach to call dom.window.close() and then remove global.window and global.document (and clear any references like parent or panel) so globals are restored between tests.packages/core-ui/src/tests/utils/options-delimited-controls.test.js (1)
10-21: 💤 Low valueConsider cleaning up global variables in afterEach.
The test sets
global.windowandglobal.documentinbeforeEachbut doesn't clean them up inafterEach. While Jest's test isolation may handle this, explicitly resetting these globals can prevent potential cross-test contamination.♻️ Optional cleanup pattern
afterEach(() => { dom.window.close(); + delete global.window; + delete global.document; });🤖 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 `@packages/core-ui/src/tests/utils/options-delimited-controls.test.js` around lines 10 - 21, The afterEach currently only closes dom; also remove the globals and test-created references to avoid cross-test leakage by clearing global.window and global.document (set to undefined or delete), and null out local refs like dom, parent, and panel so JSDOM and DelimitedOptions instances are fully torn down; update the afterEach that calls dom.window.close() to additionally unset global.window, global.document, dom, parent, and panel.packages/core-ui/src/tests/utils/options-csharp-panel.test.js (1)
28-50: ⚡ Quick winThree option properties are set but never asserted — coverage gaps.
collectionType = 'list'(Line 31),prettyPrint = true(Line 38), andprettyPrintDelimiter = '\t'(Line 39) are all set on the options object but have no correspondingexpect(...)assertions. If the implementation ever stops honouring any of these, the test will continue to pass silently.Additionally,
select[name='collectiontype']is asserted to equal'ireadonlylist'(Line 43), which maps tocollectionTargetType— notcollectionType. Confirm this is the intended DOM mapping, and consider adding assertions for the uncovered fields.🤖 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 `@packages/core-ui/src/tests/utils/options-csharp-panel.test.js` around lines 28 - 50, The test sets CSharpConvertorOptions properties collectionType, prettyPrint and prettyPrintDelimiter but never asserts them; update the test that calls panel.setFromOptions(options) to also assert the DOM reflects those values (verify select[name='collectiontype'] maps to collectionType or collectionTargetType and adjust if needed), add expects for collectionType (or change the earlier expect to assert collectionTargetType vs collectionType explicitly), add expects for the prettyPrint control (input[name='prettyprint'] checked) and for the prettyPrintDelimiter control (input[name='prettyprintdelimiter'] value === '\t'), and ensure these assertions reference the same panel.setFromOptions usage so the test fails if those mappings break.apps/api/src/options.route.test.js (1)
71-73: 💤 Low valueConsider using a more informative assertion form.
Wrapping a boolean expression in
.toBeTruthy()produces failure output likeExpected: false, Received: falsewith no indication of the actual value. Since this runs in a nested loop over 24 formats × N keys, diagnosing a failure is harder than necessary.♻️ Proposed refactor
- expect(typeof body?.tips?.[key]).toBe('string'); - expect(body.tips[key].trim().length > 0).toBeTruthy(); + expect(typeof body?.tips?.[key]).toBe('string'); + expect(body.tips[key].trim().length).toBeGreaterThan(0);🤖 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 `@apps/api/src/options.route.test.js` around lines 71 - 73, Replace the weak assertion that wraps a boolean expression with a number-based assertion to get clearer failure messages: instead of expect(body.tips[key].trim().length > 0).toBeTruthy(), assert the trimmed length directly, e.g. expect(body.tips[key].trim().length).toBeGreaterThan(0) (referencing the body.tips[key] expression in the existing test).
🤖 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 `@packages/core/package.json`:
- Line 25: The package.json "test" script currently runs only a single test file
("generateFromTextSpec.test.js"), which excludes the rest of the test suite;
update the "test" script in packages/core package.json (the "test" npm script)
to restore running the full Jest test set (e.g., remove the explicit single-file
path so it uses the root/package.json Jest config and its testMatch patterns or
use a glob that matches src/tests/**/*.test.js) so that all 52+ tests in
src/tests/core-api are executed when running npm test in packages/core.
---
Outside diff comments:
In `@packages/core-ui/src/tests/utils/options-json-panel.test.js`:
- Around line 97-109: The test reveals that JsonOptionsPanel.setFromOptions
writes undefined into the DOM causing the input[name='custom-pretty-delimiter']
to become the string "undefined"; fix setFromOptions in JsonOptionsPanel (the
code path that handles options.options.prettyPrintDelimiter) to null-guard when
assigning to DOM inputs (e.g., use value ?? '' or similar) so undefined becomes
an empty string, and update the test assertion to expect '' for
input[name='custom-pretty-delimiter'] instead of 'undefined'.
---
Nitpick comments:
In `@apps/api/src/options.route.test.js`:
- Around line 71-73: Replace the weak assertion that wraps a boolean expression
with a number-based assertion to get clearer failure messages: instead of
expect(body.tips[key].trim().length > 0).toBeTruthy(), assert the trimmed length
directly, e.g. expect(body.tips[key].trim().length).toBeGreaterThan(0)
(referencing the body.tips[key] expression in the existing test).
In `@packages/core-ui/src/tests/utils/options-csharp-panel.test.js`:
- Around line 28-50: The test sets CSharpConvertorOptions properties
collectionType, prettyPrint and prettyPrintDelimiter but never asserts them;
update the test that calls panel.setFromOptions(options) to also assert the DOM
reflects those values (verify select[name='collectiontype'] maps to
collectionType or collectionTargetType and adjust if needed), add expects for
collectionType (or change the earlier expect to assert collectionTargetType vs
collectionType explicitly), add expects for the prettyPrint control
(input[name='prettyprint'] checked) and for the prettyPrintDelimiter control
(input[name='prettyprintdelimiter'] value === '\t'), and ensure these assertions
reference the same panel.setFromOptions usage so the test fails if those
mappings break.
In `@packages/core-ui/src/tests/utils/options-csv-delimited-controls.test.js`:
- Around line 10-21: The test sets global.window and global.document in
beforeEach (creating JSDOM and instantiating CsvDelimitedOptions and calling
panel.addToGui) but afterEach only closes dom.window; update afterEach to also
delete or reset global.window and global.document (and optionally parent/panel)
to avoid cross-test contamination — locate the beforeEach/afterEach block and
modify the afterEach to call dom.window.close() and then remove global.window
and global.document (and clear any references like parent or panel) so globals
are restored between tests.
In `@packages/core-ui/src/tests/utils/options-delimited-controls.test.js`:
- Around line 10-21: The afterEach currently only closes dom; also remove the
globals and test-created references to avoid cross-test leakage by clearing
global.window and global.document (set to undefined or delete), and null out
local refs like dom, parent, and panel so JSDOM and DelimitedOptions instances
are fully torn down; update the afterEach that calls dom.window.close() to
additionally unset global.window, global.document, dom, parent, and panel.
In `@packages/core/src/tests/data_generation/unit/rulesParser.test.js`:
- Line 4: The test mixes CommonJS and ESM by using const RandExp =
require('randexp');—replace the CommonJS require with an ES module import for
the RandExp identifier (i.e., import RandExp from 'randexp') so the file
consistently uses ESM; ensure the RandExp import is used wherever the RandExp
symbol appears in the tests and that the project/test runner is configured to
support ESM if needed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 25780081-e574-4e13-897d-cb2f0d3e5e8e
⛔ Files ignored due to path filters (7)
packages/core/src/tests/data_formats/__snapshots__/test-framework-convertor.csharp.test.js.snapis excluded by!**/*.snappackages/core/src/tests/data_formats/__snapshots__/test-framework-convertor.java.test.js.snapis excluded by!**/*.snappackages/core/src/tests/data_formats/__snapshots__/test-framework-convertor.javascript.test.js.snapis excluded by!**/*.snappackages/core/src/tests/data_formats/__snapshots__/test-framework-convertor.kotlin.test.js.snapis excluded by!**/*.snappackages/core/src/tests/data_formats/__snapshots__/test-framework-convertor.perl.test.js.snapis excluded by!**/*.snappackages/core/src/tests/data_formats/__snapshots__/test-framework-convertor.php.test.js.snapis excluded by!**/*.snappackages/core/src/tests/data_formats/__snapshots__/test-framework-convertor.python.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (122)
.eslintrc.cjsapps/api/jest.config.cjsapps/api/package.jsonapps/api/src/docs.route.test.jsapps/api/src/fromschema.route.test.jsapps/api/src/generate.route.test.jsapps/api/src/options.route.test.jsapps/api/src/startup.test.jsapps/mcp/package.jsonapps/mcp/src/interop.test.jsapps/mcp/src/mcp.test.jsfinal_results.txtpackage.jsonpackages/core-ui/src/tests/generator/data-generator-page.test.jspackages/core-ui/src/tests/grid/ag-grid-main-display-grid.test.jspackages/core-ui/src/tests/grid/exporter.test.jspackages/core-ui/src/tests/grid/grid-control.test.jspackages/core-ui/src/tests/grid/grid-extension-parity.test.jspackages/core-ui/src/tests/grid/guarded-column-edits.test.jspackages/core-ui/src/tests/grid/importer.test.jspackages/core-ui/src/tests/grid/main-display-grid-selection.test.jspackages/core-ui/src/tests/grid/tabulator-duplicate-column-copy.test.jspackages/core-ui/src/tests/grid/tabulator-main-display-grid.test.jspackages/core-ui/src/tests/grid/tabulator-performance.benchmark.test.jspackages/core-ui/src/tests/grid/test-data-defn-engine-compat.test.jspackages/core-ui/src/tests/grid/testdatadefn-faker-dropdown-literals.test.jspackages/core-ui/src/tests/utils/download.test.jspackages/core-ui/src/tests/utils/drag-drop-control.test.jspackages/core-ui/src/tests/utils/export-controls.test.jspackages/core-ui/src/tests/utils/faker-command-help-metadata.test.jspackages/core-ui/src/tests/utils/grid-engine.test.jspackages/core-ui/src/tests/utils/grid-library-loader.test.jspackages/core-ui/src/tests/utils/help-tooltips.test.jspackages/core-ui/src/tests/utils/html-options-data-utils.test.jspackages/core-ui/src/tests/utils/import-export-controls-mode.test.jspackages/core-ui/src/tests/utils/options-ascii-table.test.jspackages/core-ui/src/tests/utils/options-csharp-panel.test.jspackages/core-ui/src/tests/utils/options-csv-delimited-controls.test.jspackages/core-ui/src/tests/utils/options-delimited-controls.test.jspackages/core-ui/src/tests/utils/options-gherkin-panel.test.jspackages/core-ui/src/tests/utils/options-html-panel.test.jspackages/core-ui/src/tests/utils/options-java-panel.test.jspackages/core-ui/src/tests/utils/options-javascript-panel.test.jspackages/core-ui/src/tests/utils/options-json-panel.test.jspackages/core-ui/src/tests/utils/options-kotlin-panel.test.jspackages/core-ui/src/tests/utils/options-markdown-panel.test.jspackages/core-ui/src/tests/utils/options-panel-nesting.test.jspackages/core-ui/src/tests/utils/options-perl-panel.test.jspackages/core-ui/src/tests/utils/options-php-panel.test.jspackages/core-ui/src/tests/utils/options-python-panel.test.jspackages/core-ui/src/tests/utils/options-ruby-panel.test.jspackages/core-ui/src/tests/utils/options-sql-panel.test.jspackages/core-ui/src/tests/utils/options-test-framework-panel.test.jspackages/core-ui/src/tests/utils/options-typescript-panel.test.jspackages/core-ui/src/tests/utils/options-xml-panel.test.jspackages/core-ui/src/tests/utils/tabbed-text-control-mode.test.jspackages/core-ui/src/tests/utils/test-data-amend.test.jspackages/core/package.jsonpackages/core/src/tests/core-api/generateFromTextSpec.test.jspackages/core/src/tests/data_formats/csharp-convertor.test.jspackages/core/src/tests/data_formats/csv-convertor.test.jspackages/core/src/tests/data_formats/delimiter-convertor.test.jspackages/core/src/tests/data_formats/dsv-exporter.test.jspackages/core/src/tests/data_formats/export-string-conversion-bug-fix.test.jspackages/core/src/tests/data_formats/generic-data-table-utils.test.jspackages/core/src/tests/data_formats/generic-data-table.test.jspackages/core/src/tests/data_formats/gherkin-convertor.test.jspackages/core/src/tests/data_formats/html-convertor.test.jspackages/core/src/tests/data_formats/java-convertor.test.jspackages/core/src/tests/data_formats/javascript-convertor.test.jspackages/core/src/tests/data_formats/json-convertor.test.jspackages/core/src/tests/data_formats/kotlin-convertor.test.jspackages/core/src/tests/data_formats/markdown-convertor.test.jspackages/core/src/tests/data_formats/papa-parse.test.jspackages/core/src/tests/data_formats/perl-convertor.test.jspackages/core/src/tests/data_formats/php-convertor.test.jspackages/core/src/tests/data_formats/python-convertor.test.jspackages/core/src/tests/data_formats/ruby-convertor.test.jspackages/core/src/tests/data_formats/sql-convertor.test.jspackages/core/src/tests/data_formats/test-framework-convertor-syntax.javascript.test.jspackages/core/src/tests/data_formats/test-framework-convertor-syntax.perl.test.jspackages/core/src/tests/data_formats/test-framework-convertor-syntax.php.test.jspackages/core/src/tests/data_formats/test-framework-convertor-syntax.python.test.jspackages/core/src/tests/data_formats/test-framework-convertor-syntax.ruby.test.jspackages/core/src/tests/data_formats/test-framework-convertor-syntax.shared.jspackages/core/src/tests/data_formats/test-framework-convertor.common.test.jspackages/core/src/tests/data_formats/test-framework-convertor.csharp.test.jspackages/core/src/tests/data_formats/test-framework-convertor.java.test.jspackages/core/src/tests/data_formats/test-framework-convertor.javascript.test.jspackages/core/src/tests/data_formats/test-framework-convertor.kotlin.test.jspackages/core/src/tests/data_formats/test-framework-convertor.perl.test.jspackages/core/src/tests/data_formats/test-framework-convertor.php.test.jspackages/core/src/tests/data_formats/test-framework-convertor.python.test.jspackages/core/src/tests/data_formats/test-framework-convertor.ruby.test.jspackages/core/src/tests/data_formats/test-framework-convertor.shared.jspackages/core/src/tests/data_formats/typescript-convertor.test.jspackages/core/src/tests/data_formats/xml-convertor.test.jspackages/core/src/tests/data_generation/enum-compiler-integration.test.jspackages/core/src/tests/data_generation/enum-format-bug-fix.test.jspackages/core/src/tests/data_generation/enum-integration-e2e.test.jspackages/core/src/tests/data_generation/generateDataFromFakerSpec.test.jspackages/core/src/tests/data_generation/generateDataFromMultiLineSpec.test.jspackages/core/src/tests/data_generation/pairwise-data-structure-fix.test.jspackages/core/src/tests/data_generation/pairwise.test.jspackages/core/src/tests/data_generation/quoted-enum-values.test.jspackages/core/src/tests/data_generation/unit/enum/enumTestDataGenerator.test.jspackages/core/src/tests/data_generation/unit/enum/enumTestDataRuleValidator.test.jspackages/core/src/tests/data_generation/unit/faker/fakerCommand.test.jspackages/core/src/tests/data_generation/unit/faker/fakerTestDataGenerator.test.jspackages/core/src/tests/data_generation/unit/faker/fakerTestDataRuleValidator.test.jspackages/core/src/tests/data_generation/unit/faker/hybridFakerCommandRunner.test.jspackages/core/src/tests/data_generation/unit/faker/parsingUtils.test.jspackages/core/src/tests/data_generation/unit/faker/saferFakerCommandRunner.test.jspackages/core/src/tests/data_generation/unit/rulesParser.test.jspackages/core/src/tests/data_generation/unit/testDataRules-collection.test.jspackages/core/src/tests/data_generation/unit/testDataRules.test.jspackages/core/src/tests/data_generation/user-reported-bug-http-method.test.jspackages/core/src/tests/integration/unit-test-export-parity.test.jspackages/core/src/tests/utils/debouncer.test.jspackages/core/src/tests/utils/number-converter.test.jspackages/core/src/tests/utils/papawrappa.test.jsresults.txt
💤 Files with no reviewable changes (2)
- final_results.txt
- results.txt
This was referenced May 8, 2026
Merged
This was referenced Jun 9, 2026
Merged
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
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.
closes #62
Summary by CodeRabbit