Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed functional issues in the new Vitest environment teardown and in package dependency declarations that can break test/lint execution and coverage collection.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR migrates the Lara-JS package from Jest/ESLint to Vitest with a custom Vitest environment to manage the Java/Weaver lifecycle, and replaces ESLint/Prettier tooling with oxlint/oxfmt. It also adds a packaging-time check for required Java binaries and bumps Lara-JS to v4.0.0 to reflect the tooling/test runner switch.
Changes:
- Introduces a Vitest configuration + custom environment for Weaver/
node-javashutdown handling. - Replaces Jest-based test helpers/configs with Vitest equivalents and updates affected tests.
- Switches lint/format tooling to oxlint/oxfmt and adds
validateJavaBinariesplus tests.
File summaries
| File | Description |
|---|---|
| Lara-JS/vitest/weaverVitestConfig.ts | Adds shared Vitest config for Weaver tests (coverage, environment, worker settings). |
| Lara-JS/vitest/weaverTestHelpers.ts | Minor fix to helper formatting/argument list. |
| Lara-JS/vitest/WeaverLegacyTester.ts | Migrates legacy tester from Jest spies to Vitest and adjusts dynamic imports. |
| Lara-JS/vitest/weaverEnvironment.ts | Adds a custom Vitest environment to control Weaver + node-java shutdown. |
| Lara-JS/vitest/validateJavaBinaries.test.ts | Adds unit tests for Java distribution validation logic. |
| Lara-JS/vitest.config.ts | Adds package-level Vitest config wiring in Weaver defaults. |
| Lara-JS/tsconfig.json | Switches global types from Jest to Vitest globals. |
| Lara-JS/scripts/validateJavaBinaries.ts | Adds runtime validation for packaged Java binaries directory contents. |
| Lara-JS/scripts/build-LaraJoinPoint.ts | Updates linter directives to oxlint equivalents in generated output. |
| Lara-JS/package.json | Bumps major version; swaps test/lint scripts; updates exports for vitest/oxlint configs. |
| Lara-JS/oxlint.config.ts | Adds oxlint configuration with TypeScript + Vitest plugin rules. |
| Lara-JS/jest/setupFiles/sharedJavaModule.ts | Removes Jest-only setup file. |
| Lara-JS/jest/setupFiles/importSideEffects.ts | Removes Jest-only side-effect importer. |
| Lara-JS/jest/jestGlobalTeardown.ts | Removes Jest global teardown hook. |
| Lara-JS/jest/jestGlobalSetup.ts | Removes Jest global setup hook. |
| Lara-JS/jest/jestEnvironment.ts | Removes custom Jest environment. |
| Lara-JS/jest.config.ts | Removes root Jest config. |
| Lara-JS/eslint.config.ts | Removes ESLint flat config in favor of oxlint. |
| Lara-JS/code/Weaver.ts | Updates linter directives (eslint → oxlint) while keeping Weaver logic intact. |
| Lara-JS/code/jest.config.ts | Removes Jest config under code/. |
| Lara-JS/code/JavaError.ts | Updates linter directives (eslint → oxlint). |
| Lara-JS/code/ChildProcessHandling.test.ts | Migrates mock/spies from Jest to Vitest (vi). |
| Lara-JS/api/LegacyIntegrationTests - Weaver.test.ts | Switches integration tests to Vitest-based legacy tester + updates linter directive. |
| Lara-JS/api/LegacyIntegrationTests - JsEngine.test.ts | Switches integration tests to Vitest-based legacy tester + updates linter directive. |
| Lara-JS/api/LegacyIntegrationTests - API.test.ts | Switches to Vitest legacy tester; updates failing-test marker; clears PrintOnce state. |
| Lara-JS/api/LaraJoinPoint.ts | Updates generated file linter directive (eslint → oxlint). |
| Lara-JS/api/lara/util/PrintOnce.test.ts | Adds cleanup to avoid cross-test state leakage. |
| Lara-JS/api/lara/util/JavaTypes.ts | Updates linter directives (eslint → oxlint) for namespace/empty-object-type rules. |
| Lara-JS/api/lara/util/IdGenerator.test.ts | Adds cleanup to avoid cross-test state leakage. |
| Lara-JS/api/lara/pass/results/PassResult.ts | Updates linter directive (eslint → oxlint) for namespace rule. |
| Lara-JS/api/lara/pass/AdapterPass.ts | Updates linter directive (eslint → oxlint) for namespace rule. |
| Lara-JS/api/lara/metrics/FileSizeMetric.ts | Updates linter directive (eslint → oxlint) for unused-vars rule. |
| Lara-JS/api/lara/benchmark/BenchmarkSet.ts | Updates linter directives in default callbacks (eslint → oxlint). |
| Lara-JS/api/lara/benchmark/BenchmarkInstance.ts | Updates linter directives in default callbacks (eslint → oxlint). |
| Lara-JS/api/jest.config.ts | Removes Jest config under api/. |
| Lara-JS/.npmrc | Enables engine-strict for consistent toolchain installs. |
| .vscode/settings.json | Removes Jest VS Code configuration. |
| .vscode/extensions.json | Removes Jest extension recommendation. |
Review details
- Files reviewed: 38/38 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Replace Jest and ts-jest with Vitest's native Node TypeScript runner, add shared Vitest setup helpers, and configure Vitest-aware linting and Prettier.
Replace ESLint and Prettier scripts/configuration with Oxlint and Oxfmt, retain TSDoc validation through the compatible plugin bridge, and update lint-disable directives.
Run Lara tests in one shared fork with a Weaver environment that initializes before every test and stops Java only after Vitest has completed worker reporting. Replace per-project setup/config files and preserve legacy file isolation.
Expose a fixed-path validator for weaver package distributions and cover missing, symlinked, malformed, and valid java-binaries layouts.
lm-sousa
force-pushed
the
vitest
branch
2 times, most recently
from
September 5, 2026 02:04
a6c5aee to
da7df06
Compare
- Fix coverage include glob: the Jest-era **/*[^.d].(t|j)s never matched any file (bare parens are not an extglob); use **/*.{t,j}s.
- Remove the console.Console swap from the weaver environment: no historical rationale and nothing depends on it. Vitest requires setup to return EnvironmentReturn, so setup returns a no-op teardown.
- Make the weaver environment path portable: Vitest only treats names starting with "." or "/" as file paths and resolves them with pathe, so a "/"-prefixed drive path fixes Windows (ERR_UNSUPPORTED_ESM_URL_SCHEME).
Full Vitest suite verified locally and in an Ubuntu 24.04 container; CI green on all platforms.
|
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.
No description provided.