fix: remove deprecation warning from uloop update

[hatayama]

Summary

• remove shell mode from npm commands used by uloop update and version detection
• add a regression test that verifies the update flow does not enable shell mode
• gate the CLI entry point during Jest runs so the update helpers can be tested safely

Testing

• npm run test:cli -- --runTestsByPath src/tests/cli-update.test.ts
• npm run lint
• npm run build

---

Summary by cubic

Remove shell mode from npm invocations in uloop update and version detection to eliminate the deprecation warning and run commands directly. Adds a regression test and guards the CLI entry point under Jest so update helpers can be tested safely.

^{Written for commit 80c4e85. Summary will update on new commits.}

Overview

This PR removes the shell: true option from npm commands in the CLI update workflow and adds regression tests to verify the fix. The changes prevent deprecation warnings from npm while maintaining functional equivalence of the command invocations.

Changes

Test Coverage

• New test file: Packages/src/Cli~/src/__tests__/cli-update.test.ts (+129 lines)
  • Adds comprehensive tests for the update flow with mocked child_process.spawn
  • Verifies getInstalledVersion() correctly parses npm list output and invokes the callback with the resolved version
  • Verifies updateCli() spawns the global install command without enabling shell mode
  • Explicitly asserts that spawn options do not include a shell property
  • Mocks launch-unity module to isolate update logic testing
  • Resets mocks and suppresses console output between tests

Core Changes

• Packages/src/Cli~/src/cli.ts (+6/-7 lines)
  • Exported getInstalledVersion() and updateCli() functions to enable testing
  • Removed shell: true from both spawn() calls in getInstalledVersion() and updateCli()
  • Added conditional check to prevent CLI entry point execution during Jest test runs (process.env.JEST_WORKER_ID === undefined)

Minor Updates

• Packages/src/Cli~/src/project-root.ts (+1/-1 lines)
  • Formatting: Updated arrow function callback in hasUloopInstalled() to use consistent parentheses syntax

Impact

• Eliminates deprecation warnings from npm while preserving command functionality
• Enables safe testing of update helpers by preventing CLI initialization during Jest runs
• Maintains backward compatibility with existing behavior

[coderabbitai]

📝 Walkthrough

Walkthrough

This change adds test coverage for CLI update functionality and refactors the CLI implementation to remove shell mode from npm spawn calls while exporting previously private functions for testing purposes.

Changes

Cohort / File(s)	Summary
Test Suite Packages/src/Cli~/src/__tests__/cli-update.test.ts	New Jest test file verifying spawn behavior for npm commands. Tests getInstalledVersion() and updateCli() flows with mocked child processes and launch-unity module, explicitly asserting shell mode is not enabled.
CLI Core Implementation Packages/src/Cli~/src/cli.ts	Exports getInstalledVersion and updateCli functions; removes shell: true from spawn calls for npm commands; adds conditional to skip main() execution when running under Jest worker.
Project Root Utility Packages/src/Cli~/src/project-root.ts	Minor arrow function formatting: adds parentheses around arrow parameter for consistency.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: remove deprecation warning from uloop update' accurately describes the main change in the PR, which removes shell mode from npm commands to eliminate a deprecation warning, as confirmed by the file changes and PR description.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/hatayama/fix-update-shell-warning

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

Packages/src/Cli~/src/__tests__/cli-update.test.ts (1)

100-128: Consider adding test cases for error paths.

The test effectively verifies the success path and confirms no shell option (line 114). However, consider adding test cases for:

• Non-zero exit code from install
• Spawn error event

These would improve coverage of the error handling in updateCli().

💡 Optional: Add error path test cases

it('handles non-zero exit code from npm install', () => {
  const updateChild = createMockChildProcess();
  const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => undefined as never);
  jest.spyOn(console, 'error').mockImplementation(() => {});
  mockSpawn.mockReturnValue(updateChild);

  updateCli();
  updateChild.emitClose(1);

  expect(exitSpy).toHaveBeenCalledWith(1);
});

it('handles spawn error', () => {
  const updateChild = createMockChildProcess();
  const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => undefined as never);
  jest.spyOn(console, 'error').mockImplementation(() => {});
  mockSpawn.mockReturnValue(updateChild);

  updateCli();
  updateChild.emitError(new Error('ENOENT'));

  expect(exitSpy).toHaveBeenCalledWith(1);
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Packages/src/Cli`~/src/__tests__/cli-update.test.ts around lines 100 - 128,
Add two tests under the existing suite exercising error paths for updateCli():
one that simulates a non-zero install exit and one that simulates a spawn error.
For each test use createMockChildProcess() and mockSpawn.mockReturnValue(...)
(or mockReturnValueOnce if ordering matters), spy on process.exit
(mockImplementation to avoid real exit) and stub console.error, call
updateCli(), then for the first test call updateChild.emitClose(1) and assert
process.exit was called with 1, and for the second call
updateChild.emitError(new Error('ENOENT')) and assert process.exit was called
with 1; reference updateCli, mockSpawn, createMockChildProcess, emitClose and
emitError to locate code to change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@Packages/src/Cli`~/src/__tests__/cli-update.test.ts:
- Around line 100-128: Add two tests under the existing suite exercising error
paths for updateCli(): one that simulates a non-zero install exit and one that
simulates a spawn error. For each test use createMockChildProcess() and
mockSpawn.mockReturnValue(...) (or mockReturnValueOnce if ordering matters), spy
on process.exit (mockImplementation to avoid real exit) and stub console.error,
call updateCli(), then for the first test call updateChild.emitClose(1) and
assert process.exit was called with 1, and for the second call
updateChild.emitError(new Error('ENOENT')) and assert process.exit was called
with 1; reference updateCli, mockSpawn, createMockChildProcess, emitClose and
emitError to locate code to change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 894e5651-df2f-4d2c-9192-a407e262654c

📥 Commits

Reviewing files that changed from the base of the PR and between 5f55dcd and 80c4e85.

📒 Files selected for processing (3)

• Packages/src/Cli~/src/__tests__/cli-update.test.ts
• Packages/src/Cli~/src/cli.ts
• Packages/src/Cli~/src/project-root.ts

[cubic-dev-ai]

No issues found across 3 files