-
Notifications
You must be signed in to change notification settings - Fork 294
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
Development
: Fix running server tests on Windows
#9193
Conversation
WalkthroughThe changes across the Java files enhance the overall robustness and maintainability of key functionalities. Key loading, resource management, and file permission handling have been improved to prevent potential exceptions and ensure compatibility across different operating systems. These modifications employ defensive programming techniques, ensuring that the application behaves predictably in the face of unexpected input or environmental conditions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Application
participant Repository
User->>Application: Initiate SSH Command
Application->>Repository: Open Repository
alt Successful Operation
Repository-->>Application: Repository opened
Application->>User: Command executed successfully
else Error
Repository-->>Application: Throw exception
Application->>User: Handle error
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good to me. Thanks for the fix! 👍
The test @bensofficial Any idea what's causing this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, tests pass on windows 10, code lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests pass for me on Windows 10 👍
I had a look at the server logs and it seems like the repository does not exist or is not properly created.
The build agent tries to clone the repository, but apparently the server cannot find the folder of the repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code
Checklist
General
Motivation and Context
After #8951 got merged it was no longer possible to run the LocalVC server tests locally. This PR fixes this issue.
Description
This was caused by two issues.
Development
: Add support for ephemeral SSH keys for the authentication of build agents #8951 expected the file system to support posix permissions, which is not true on WindowsIntegrated code lifecycle
: Add option to clone repositories using SSH #8110 was not properly closing the Repository, leading to a failure in the post-test cleanup,Note: The test
testPriorityRunningExam
is consistently failing since the merge of PR #8951. This is some issue related to the GitHub-Runners and does not happen locally on any OS. This PR does not fix this issue.Steps for Testing
run the
LocalVCLocalCIIntegrationTest
tests locally and see that they pass.Review Progress
Code Review
Summary by CodeRabbit
Bug Fixes
Refactor