Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command fixes #728

Merged
merged 5 commits into from
Feb 4, 2025
Merged

Conversation

nissa-seru
Copy link

@nissa-seru nissa-seru commented Feb 2, 2025

Implements cline@5fd60b7

I see that prettier also decided to prettify tsconfig.json but no actual changes.


Important

Adds cross-platform shell detection utility and updates command execution descriptions with comprehensive tests.

  • Shell Detection:
    • Introduces getShell() in shell.ts for cross-platform shell detection using VS Code config, userInfo(), and environment variables.
    • Handles Windows, macOS, Linux, and unknown platforms with specific fallbacks.
  • Testing:
    • Adds shell.test.ts for testing getShell() across different platforms and error scenarios.
  • Command Execution:
    • Updates getExecuteCommandDescription() in execute-command.ts to include shell-specific command chaining syntax.
  • System Info:
    • Replaces defaultShell with getShell() in system-info.ts for accurate shell information.

This description was created by Ellipsis for f8b29e7. It will automatically update as commits are pushed.

Copy link

changeset-bot bot commented Feb 2, 2025

⚠️ No Changeset found

Latest commit: 7e5d78d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mrubens
Copy link
Collaborator

mrubens commented Feb 3, 2025

Ugh, I updated the snapshots - not sure why the tests are still failing 🤔

@mrubens
Copy link
Collaborator

mrubens commented Feb 3, 2025

I realized that the remaining tests are failing because they don't all run locally (so the snapshots aren't being updated), at least for me.

Determining test suites to run...
Found 56 test suites
............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. Active timers can also cause this, ensure that .unref() was called on them.

Ran 748 tests in 3.242 s
 748 passing 0 failing 0 pending

Versus 756 tests run in CI. I'm sure unrelated to this test, but we should get to the bottom of whatever is going on here.

@nissa-seru
Copy link
Author

I strongly suspect that this is related to something in Cline.test.ts - when I was originally setting up dev environment for Roo, a very error came up and I wasn't able to address it satisfactorily. It would manifest as an uncaught error in Cline.tsline 762 throw new Error("Unexpected: No existing API conversation history") when running tests, but cleaning up the exception handling for relevant tests revealed that the tests would create a zombie process that required forceful cleanup if the exception was not thrown.

My synthesis of this would be "increment priority of Cline.test.ts cleanup"

@mrubens
Copy link
Collaborator

mrubens commented Feb 4, 2025

I realized that the remaining tests are failing because they don't all run locally (so the snapshots aren't being updated), at least for me.

Determining test suites to run...
Found 56 test suites
............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. Active timers can also cause this, ensure that .unref() was called on them.

Ran 748 tests in 3.242 s
 748 passing 0 failing 0 pending

Versus 756 tests run in CI. I'm sure unrelated to this test, but we should get to the bottom of whatever is going on here.

Actually I was wrong - the failing tests were ones in origin/main that hadn't yet been merged into this branch. I am super confused about why they were running in CI 🤔

@mrubens mrubens merged commit 8472ae0 into RooVetGit:main Feb 4, 2025
6 checks passed
@nissa-seru nissa-seru deleted the add-command-fixes-from-cline branch February 8, 2025 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants