test: improve integration test by 16bit-ykiko · Pull Request #246 · clice-io/clice

16bit-ykiko · 2025-09-08T14:58:34Z

No description provided.

gemini-code-assist

Summary of Changes

Hello @16bit-ykiko, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on enhancing the stability and reliability of the integration test suite. The primary goal is to improve the lifecycle management of the Language Server Protocol (LSP) server processes during testing, ensuring they are properly started, stopped, and cleaned up, even in error scenarios. Additionally, it refines how test resources are handled, making the test setup more adaptable.

Highlights

Improved Test Teardown: Enhanced the LSPTransport's stop method for more robust and graceful shutdown of the LSP server, including proper task cancellation, process termination, and error handling.
Flexible Resource Directory Handling: Modified the resource_dir fixture and its usage to allow for optional resource directories, preventing errors when not provided.
Streamlined Client Exit: Refactored the client exit logic to ensure stop() is called once and effectively, preventing redundant calls and improving cleanup.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

The pull request improves the integration tests by making the resource directory optional, ensuring the LSP client is properly stopped, and enhancing error handling during the transport process. The changes include adding type hints, conditional logic for resource directory, task management for transport, and improved stream handling.

16bit-ykiko · 2025-09-08T16:18:15Z

Close #227.

16bit-ykiko · 2025-09-08T17:52:15Z

/gemini summary

16bit-ykiko · 2025-09-08T17:52:32Z

/gemini review

gemini-code-assist · 2025-09-08T17:52:48Z

Summary of Changes

This pull request significantly enhances the testing and debugging capabilities of the project by introducing flexible communication modes for integration tests and providing comprehensive documentation for these new features. The changes aim to streamline the development workflow by making it easier to run tests and debug the application in various environments.

Highlights

Enhanced Integration Test Infrastructure: The Python-based integration test harness has been significantly improved to support both pipe and socket communication modes, allowing for more flexible testing and debugging scenarios. This includes adding new command-line options to the clice executable for specifying host and port in socket mode.
Refactored Test Documentation: Detailed instructions for running unit and integration tests, as well as debugging clice (especially with VS Code in socket mode), have been moved to new dedicated documentation files (test-and-debug.md) in both English and Chinese. Existing build and contribution guides have been updated to link to these new resources.
Robust LSP Client in Tests: The internal LSP client used in the Python test suite (LSPTransport) has been made more robust. It now includes better handling for process lifecycle, graceful shutdown, task management, and error handling during message reading and sending, ensuring more reliable and clean test execution.

Changelog

CMakeLists.txt
- Removed an extraneous blank line.
docs/en/dev/build.md
- Removed outdated test execution instructions, directing users to the new test-and-debug.md.
- Corrected markdown formatting for GCC toolchain example by adding a closing code block tag.
docs/en/dev/contribution.md
- Updated the link to include the new test-and-debug.md documentation.
docs/en/dev/test-and-debug.md
- Added a new document detailing how to run unit and integration tests.
- Included instructions for debugging clice using socket mode, including pytest integration and VS Code setup.
docs/zh/dev/build.md
- Removed outdated test execution instructions (Chinese version).
- Corrected markdown formatting for GCC toolchain example by adding a closing code block tag (Chinese version).
docs/zh/dev/contribution.md
- Updated the link to include the new test-and-debug.md documentation (Chinese version).
docs/zh/dev/test-and-debug.md
- Added a new document (Chinese version) detailing how to run unit and integration tests.
- Included instructions for debugging clice using socket mode, including pytest integration and VS Code setup (Chinese version).
include/Async/Network.h
- Renamed the ip parameter to host in the listen function signature for better clarity.
src/Async/Network.cpp
- Updated the listen function implementation to use host instead of ip for consistency.
src/Driver/clice.cc
- Introduced new command-line options --host and --port for specifying the network interface and port when running in socket mode.
- Refactored cl::opt declarations to use brace initialization for improved readability.
- Consolidated logging options under the main clice options category.
- Modified the main function to utilize the new --host and --port options when clice is run in socket mode.
tests/conftest.py
- Added --mode, --host, and --port command-line options for pytest to control the LSP client connection.
- Made --executable and --resource-dir options optional, allowing tests to run without them if not strictly required.
- Updated the executable and resource_dir fixtures to handle cases where paths are not provided.
- Modified the client fixture to dynamically construct the clice command based on the selected connection mode and provided host/port/resource directory.
- Removed the logging import as it was no longer directly used in this file.
tests/fixtures/client.py
- Updated the LSPClient constructor to directly accept mode, host, and port parameters, simplifying its initialization.
- Removed the redundant await self.stop() call from the exit method, as the client's shutdown is now managed by the pytest fixture's teardown.
tests/fixtures/transport.py
- Introduced LSPError for more specific error handling within the LSP transport layer.
- Updated the LSPTransport constructor to directly use the provided mode, host, and port, and to initialize a dedicated logger.
- Changed the default connection mode from stdio to pipe for clarity.
- Enhanced the start method to log the process PID and to manage asyncio.Task objects more effectively for graceful shutdown.
- Implemented a _monitor_process task to detect and react to the LSP server's exit.
- Improved the stop method to ensure all tasks and pending requests are cancelled, and to forcefully terminate the server process if it doesn't shut down gracefully within a timeout.
- Refactored _read_stderr and _read_messages for more robust error handling, including asyncio.CancelledError, ConnectionResetError, and BrokenPipeError.
- Improved message parsing in _read_messages for better header handling.
- Enhanced _handle_response and _handle_notification to be more resilient to missing IDs or parameters.
- Strengthened _send_message with checks for writer availability and improved error logging for connection issues.
- Modified send_request to use asyncio.get_running_loop().create_future() for creating futures.
- Updated the type hint for register_notification_handler to correctly reflect coroutine handlers.

Activity

16bit-ykiko opened the pull request and linked it to issue Improving the integration test framework #227.
gemini-code-assist[bot] provided several high and medium priority suggestions, including: cancelling all tasks and pending requests in stop method, adding a timeout to self.process.wait(), adding type hints, conditionally adding --resource-dir, removing redundant await self.stop() from exit method, storing tasks for better management, raising exceptions for non-zero server exit codes, and calling cancel_all() when reader is unavailable or header line is empty.
16bit-ykiko requested a summary from the bot.

gemini-code-assist

Code Review

This pull request significantly improves the integration testing framework by introducing support for different connection modes (pipe and socket) via command-line arguments. The test transport layer (LSPTransport) has been substantially refactored for better robustness, error handling, and resource management. Additionally, the documentation has been reorganized, moving testing and debugging instructions into a dedicated file for better clarity. The C++ side has been updated to support the new host and port command-line options. Overall, these are great improvements to the project's testing capabilities and developer experience. I've found a couple of minor issues in the documentation and test setup.

16bit-ykiko · 2025-09-08T18:00:16Z

/gemini review

gemini-code-assist

Code Review

This pull request significantly improves the integration testing framework by adding support for both pipe and socket communication modes. This is a great enhancement for debugging. The changes include adding command-line options to both the clice executable and the pytest test runner, and refactoring the documentation to provide clearer instructions for testing and debugging. The Python test client, particularly LSPTransport, has been substantially refactored for better robustness, error handling, and graceful shutdown, which is excellent. I've identified a couple of issues in the test setup that could lead to crashes or dangling processes, along with a minor documentation improvement.

Stop client when server has exited.

887763c

gemini-code-assist Bot reviewed Sep 8, 2025

View reviewed changes

16bit-ykiko added 3 commits September 8, 2025 23:01

Fix format.

e414046

monitor subprocess.

cae33a3

add host and port.

5a6ef2a

Update docs.

2d66312

Fix format.

cb8d048

gemini-code-assist Bot reviewed Sep 8, 2025

View reviewed changes

Comment thread tests/conftest.py

Comment thread docs/en/dev/test-and-debug.md Outdated

Fix docs.

72be17a

gemini-code-assist Bot reviewed Sep 8, 2025

View reviewed changes

Comment thread tests/conftest.py

Comment thread tests/fixtures/client.py

Comment thread docs/en/dev/test-and-debug.md

16bit-ykiko merged commit f658769 into main Sep 8, 2025
9 checks passed

16bit-ykiko deleted the integration-test branch September 8, 2025 18:04

16bit-ykiko changed the title ~~Improve integration test~~ test: improve integration test Apr 6, 2026

Conversation

16bit-ykiko commented Sep 8, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

16bit-ykiko commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

16bit-ykiko commented Sep 8, 2025

Uh oh!

16bit-ykiko commented Sep 8, 2025

Uh oh!

gemini-code-assist Bot commented Sep 8, 2025

Summary of Changes

Highlights

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

16bit-ykiko commented Sep 8, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

16bit-ykiko commented Sep 8, 2025 •

edited

Loading