Skip to content

Commit f4abce5

Browse files
devinat1greptile-apps[bot]
devinat1
and
greptile-apps[bot]
authored
feat: CI pipeline for tests regarding HQL security and clickhouse wrapper (#4700)
* feat: hql test ci * chore: update branch for testing * chore: update cache-dependency-path in CI workflow to use glob pattern for yarn.lock * feat: hql and clickhouse CI * refactor: disaggregated CI pipelines for HQL and clickhouse tests * misc: rename to remove hql * chore: remove extra branch * Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
1 parent 5fd068c commit f4abce5

File tree

4 files changed

+188
-4
lines changed

4 files changed

+188
-4
lines changed

‎.github/workflows/hql-clickhouse-tests.yml‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
name: ClickHouse Wrapper Tests
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
- devin/eng-3012-ci
8+
pull_request:
9+
branches:
10+
- main
11+
paths:
12+
# ClickHouse wrapper and database implementation files
13+
- "valhalla/jawn/src/lib/db/ClickhouseWrapper.ts"
14+
- "valhalla/jawn/src/managers/HeliconeSqlManager.ts"
15+
# ClickHouse wrapper test files
16+
- "valhalla/jawn/src/lib/db/test/TestClickhouseWrapper.test.ts"
17+
- "valhalla/jawn/src/lib/db/test/TestClickhouseWrapper.ts"
18+
- "valhalla/jawn/src/lib/db/test/clickhouseMockData.ts"
19+
# Workflow file itself
20+
- ".github/workflows/hql-clickhouse-tests.yml"
21+
22+
jobs:
23+
hql-clickhouse-wrapper-tests:
24+
name: HQL ClickHouse Wrapper Tests
25+
runs-on: ubuntu-latest
26+
defaults:
27+
run:
28+
working-directory: valhalla/jawn
29+
services:
30+
clickhouse:
31+
image: clickhouse/clickhouse-server:latest
32+
ports:
33+
- 8123:8123
34+
- 9000:9000
35+
options: >-
36+
--health-cmd "wget --no-verbose --tries=1 --spider http://localhost:8123/ping || exit 1"
37+
--health-interval 10s
38+
--health-timeout 5s
39+
--health-retries 5
40+
steps:
41+
- uses: actions/checkout@v4
42+
43+
- name: Setup Node.js
44+
uses: actions/setup-node@v4
45+
with:
46+
node-version: "20"
47+
cache: "yarn"
48+
cache-dependency-path: "**/yarn.lock"
49+
50+
- name: Install dependencies
51+
run: |
52+
# Retry logic for yarn install with fallback to npm
53+
for i in 1 2 3; do
54+
echo "Attempt $i: Installing dependencies..."
55+
if yarn install --frozen-lockfile --network-timeout 100000; then
56+
echo "✓ Successfully installed dependencies"
57+
exit 0
58+
fi
59+
echo "Attempt $i failed, waiting 5 seconds..."
60+
sleep 5
61+
done
62+
# Fallback to npm if yarn fails
63+
echo "Yarn failed, using npm install..."
64+
npm install
65+
66+
- name: Wait for ClickHouse
67+
run: |
68+
for i in {1..30}; do
69+
if curl -s http://localhost:8123/ping > /dev/null; then
70+
echo "ClickHouse is ready"
71+
break
72+
fi
73+
echo "Waiting for ClickHouse... ($i/30)"
74+
sleep 2
75+
if [ $i -eq 30 ]; then
76+
echo "ClickHouse failed to start after 30 attempts"
77+
exit 1
78+
fi
79+
done
80+
81+
- name: Run ClickHouse Wrapper Tests
82+
run: yarn test:jawn TestClickhouseWrapper.test.ts
83+
env:
84+
NODE_ENV: test
85+
CLICKHOUSE_HOST: localhost
86+
CLICKHOUSE_PORT: 8123
87+
CLICKHOUSE_USER: default
88+
CLICKHOUSE_PASSWORD: ""
89+
CLICKHOUSE_DATABASE: default

‎.github/workflows/hql-security-tests.yml‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
name: HQL Security Tests
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
pull_request:
8+
branches:
9+
- main
10+
paths:
11+
# HQL security-related implementation files
12+
- "valhalla/jawn/src/lib/stores/HqlStore.ts"
13+
- "valhalla/jawn/src/lib/errors/HqlErrors.ts"
14+
- "valhalla/jawn/src/managers/HqlQueryManager.ts"
15+
- "valhalla/jawn/src/controllers/public/heliconeSqlController.ts"
16+
# HQL security test files
17+
- "valhalla/jawn/src/lib/db/test/hqlSecurityTests.test.ts"
18+
- "valhalla/jawn/src/lib/db/test/MockClickhouseWrapper.ts"
19+
- "valhalla/jawn/src/controllers/public/__tests__/heliconeSqlController.test.ts"
20+
# Web HQL files
21+
- "web/lib/api/hql/**"
22+
# Workflow file itself
23+
- ".github/workflows/hql-security-tests.yml"
24+
25+
jobs:
26+
hql-security-tests:
27+
hql-security-tests:
28+
name: Run HQL Security Tests
29+
defaults:
30+
run:
31+
working-directory: valhalla/jawn
32+
steps:
33+
- uses: actions/checkout@v4
34+
35+
- name: Setup Node.js
36+
uses: actions/setup-node@v4
37+
with:
38+
node-version: "20"
39+
cache: "yarn"
40+
cache-dependency-path: "**/yarn.lock"
41+
42+
- name: Install dependencies
43+
run: |
44+
# Retry logic for yarn install with fallback to npm
45+
for i in 1 2 3; do
46+
echo "Attempt $i: Installing dependencies..."
47+
if yarn install --frozen-lockfile --network-timeout 100000; then
48+
echo "✓ Successfully installed dependencies"
49+
exit 0
50+
fi
51+
echo "Attempt $i failed, waiting 5 seconds..."
52+
sleep 5
53+
done
54+
# Fallback to npm if yarn fails
55+
echo "Yarn failed, using npm install..."
56+
npm install
57+
58+
- name: Run HQL Security Tests
59+
run: yarn test:jawn hqlSecurityTests.test.ts
60+
env:
61+
NODE_ENV: test
62+
63+
# Note: API integration tests are skipped in CI because they require
64+
# the full Jawn server to be running on port 8585.
65+
# These tests should be run locally or in a more complete E2E test environment.
66+
# To run locally:
67+
# 1. Start the Jawn server: yarn start
68+
# 2. Run tests: yarn test:jawn heliconeSqlController.test.ts

‎valhalla/jawn/src/lib/db/test/TestClickhouseWrapper.ts‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,19 @@ export class TestClickhouseClientWrapper {
1919
private clickHouseHqlClient: ClickHouseClient;
2020

2121
constructor(env: ClickhouseEnv) {
22+
// Use url instead of deprecated host
23+
const clickhouseUrl = env.CLICKHOUSE_HOST.startsWith('http')
24+
? env.CLICKHOUSE_HOST
25+
: `http://${env.CLICKHOUSE_HOST}`;
26+
2227
this.clickHouseClient = createClient({
23-
host: env.CLICKHOUSE_HOST,
28+
url: clickhouseUrl,
2429
username: env.CLICKHOUSE_USER,
2530
password: env.CLICKHOUSE_PASSWORD,
2631
});
2732

2833
this.clickHouseHqlClient = createClient({
29-
host: env.CLICKHOUSE_HOST,
34+
url: clickhouseUrl,
3035
username: env.CLICKHOUSE_HQL_USER,
3136
password: env.CLICKHOUSE_HQL_PASSWORD,
3237
});
@@ -193,6 +198,27 @@ export class TestClickhouseClientWrapper {
193198
}
194199
}
195200

201+
async hqlQueryWithContext<T>({
202+
query,
203+
organizationId,
204+
parameters,
205+
schema,
206+
}: {
207+
query: string;
208+
organizationId: string;
209+
parameters: (number | string | boolean | Date)[];
210+
schema?: ZodType<T>;
211+
}): Promise<Result<T[], string>> {
212+
// For HQL queries, delegate to the regular queryWithContext
213+
// since the test wrapper handles both regular and HQL queries the same way
214+
return this.queryWithContext<T>({
215+
query,
216+
organizationId,
217+
parameters,
218+
schema,
219+
});
220+
}
221+
196222
async createTestDatabase(): Promise<Result<null, string>> {
197223
try {
198224
await this.clickHouseClient.query({

‎valhalla/jawn/src/managers/HeliconeSqlManager.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,9 @@ export class HeliconeSqlManager {
220220
const elapsedMilliseconds = Date.now() - start;
221221

222222
if (isError(result)) {
223-
const errorCode = parseClickhouseError(result.error);
224-
return hqlError(errorCode, result.error);
223+
const errorString = String(result.error);
224+
const errorCode = parseClickhouseError(errorString);
225+
return hqlError(errorCode, errorString);
225226
}
226227

227228
return ok({

0 commit comments

Comments
 (0)