Skip to content

tests: update integration tests#2195

Merged
Yuan325 merged 2 commits into
config-branchfrom
config-int-test
Dec 19, 2025
Merged

tests: update integration tests#2195
Yuan325 merged 2 commits into
config-branchfrom
config-int-test

Conversation

@Yuan325

@Yuan325 Yuan325 commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

update integration tests with the decoupling of source from tool.

@Yuan325 Yuan325 requested a review from a team December 17, 2025 23:55
@gemini-code-assist

gemini-code-assist Bot commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @Yuan325, I'm Gemini Code Assist1! 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 significantly updates the ClickHouse integration tests to align with a recent architectural change that decouples data sources from tools. The tests now simulate real-world interaction by making HTTP API calls to a locally running server that exposes the ClickHouse tools, rather than directly invoking Go objects. This ensures a more robust and realistic testing environment for the toolchain.

Highlights

  • Refactored Integration Tests: The ClickHouse integration tests have been completely overhauled to interact with tools via HTTP API endpoints, simulating real-world usage.
  • Decoupling of Source from Tool: The tests now accurately reflect the architectural change where data sources are decoupled from tools, as described in the PR description.
  • Removed Direct Tool Invocation: Previous direct Go object instantiation and invocation of tools within tests have been replaced with HTTP requests to a local test server.
  • New HTTP Testing Utilities: The codebase now utilizes bytes, encoding/json, and net/http imports, along with tests.StartCmd and tests.RunRequest utilities, to facilitate this new HTTP-based testing approach.
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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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.

Footnotes

  1. 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[bot]
gemini-code-assist Bot reviewed Dec 17, 2025
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the ClickHouse integration tests to be more end-to-end by running the toolbox server and making HTTP requests to it. This is a great improvement. However, I've found several critical issues in the new test implementations, mostly related to incorrect source names in tool definitions, improperly formatted request bodies, and incorrect test logic for negative cases. These issues will likely cause the tests to fail. Please see my detailed comments for suggestions on how to fix them.

Comment thread tests/clickhouse/clickhouse_integration_test.go
Comment thread tests/clickhouse/clickhouse_integration_test.go Outdated
Comment thread tests/clickhouse/clickhouse_integration_test.go Outdated
Comment thread tests/clickhouse/clickhouse_integration_test.go Outdated
Comment thread tests/clickhouse/clickhouse_integration_test.go Outdated
Comment thread tests/clickhouse/clickhouse_integration_test.go
Comment thread tests/clickhouse/clickhouse_integration_test.go Outdated
@Yuan325 Yuan325 force-pushed the config-int-test branch 3 times, most recently from 08c1555 to fc66740 Compare December 18, 2025 00:06
@Yuan325

Yuan325 commented Dec 18, 2025

Copy link
Copy Markdown
Contributor Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Dec 18, 2025
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a great improvement, refactoring the ClickHouse integration tests to be true integration tests that exercise the server's HTTP API. This approach more accurately reflects real-world usage. The adoption of table-driven tests also enhances readability and maintainability. I've identified two instances in negative test cases where the logic is inverted, which could lead to tests passing on failure and failing on success. My review includes suggestions to correct this logic.

Comment thread tests/clickhouse/clickhouse_integration_test.go Outdated
Comment thread tests/clickhouse/clickhouse_integration_test.go Outdated
@Yuan325 Yuan325 force-pushed the config-int-test branch 8 times, most recently from 7728a7d to 86480ea Compare December 18, 2025 02:35
@Yuan325 Yuan325 requested a review from a team December 18, 2025 02:35
@Yuan325 Yuan325 force-pushed the config-merge-conflicts branch from 76a7775 to bf985f3 Compare December 18, 2025 06:09
@Yuan325 Yuan325 requested review from a team December 18, 2025 06:09
Base automatically changed from config-merge-conflicts to config-branch December 19, 2025 01:42
@Yuan325 Yuan325 merged commit c5a294e into config-branch Dec 19, 2025
11 checks passed
@Yuan325 Yuan325 deleted the config-int-test branch December 19, 2025 01:42
Yuan325 added a commit that referenced this pull request Dec 19, 2025
@Yuan325
tests: update integration tests (#2195)
cb27866 
update integration tests with the decoupling of source from tool.
Yuan325 added a commit that referenced this pull request Dec 19, 2025
@Yuan325
tests: update integration tests (#2195)
8082166 
update integration tests with the decoupling of source from tool.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@averikitsch averikitsch averikitsch approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@duwenxin99 duwenxin99

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Yuan325 @averikitsch @duwenxin99