Add bin/clickhouse-client shim symlink for upstream test runner#736
Conversation
Agent-Logs-Url: https://github.com/ClickHouse/clickhouse-js/sessions/dcf1f51c-ed87-4200-a389-343ce418e8ff Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR fixes upstream ClickHouse SQL test runner compatibility by ensuring that clickhouse-client invocations resolve to this repo’s clickhouse-test-runner shim (rather than requiring/bypassing a system-installed client), and documents the requirement.
Changes:
- Add a
bin/clickhouse-cliententry that resolves to the existingbin/clickhousewrapper so the upstream runner can execute queries via the shim. - Update the upstream test runner README to explain why both
clickhouseandclickhouse-clientmust be onPATH.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/clickhouse-test-runner/README.md | Documents the clickhouse-client resolution behavior and why both binary names must be available on PATH for the upstream runner. |
| tests/clickhouse-test-runner/bin/clickhouse-client | Provides the clickhouse-client name expected by the upstream runner, resolving to the same shim behavior as bin/clickhouse. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Summary
run-upstream-tests.shprependstests/clickhouse-test-runner/bin/toPATH, but that directory only containedclickhouse. The upstreamtests/clickhouse-testrunner shells out toclickhouse-clientto execute queries, so those invocations either bypassed the shim or failed when no real client was installed — contradicting the README's claim that the harness is aclickhouse-clientreplacement.bin/clickhouse-clientsymlink →clickhouse, so both binary names resolve to the same wrapper. The existing shim already routesextract-from-configcorrectly and forwards everything else tonode dist/main.js, so no script changes are needed.PATH(clickhouse extract-from-configduring setup,clickhouse-clientfor queries).Checklist