Skip to content

Commit 3657ae9

Browse files
ZeekoZhuZeekoZhu
authored
Merge pull request #1873 from narrowizard/fix-test
Fix Tests and Optimize Code
2 parents bf0e64e + d554145 commit 3657ae9

File tree

15 files changed

+299
-64
lines changed

15 files changed

+299
-64
lines changed

‎.github/workflows/test.yml‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
name: Tests
2+
3+
on:
4+
push:
5+
branches: ['main']
6+
pull_request:
7+
branches: ['main']
8+
9+
concurrency:
10+
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
11+
cancel-in-progress: true
12+
13+
jobs:
14+
api-tests:
15+
runs-on: ubuntu-latest
16+
timeout-minutes: 30
17+
18+
services:
19+
postgres:
20+
image: postgres:17
21+
env:
22+
POSTGRES_USER: devtable
23+
POSTGRES_PASSWORD: devtable
24+
POSTGRES_DB: devtable_test
25+
options: >-
26+
--health-cmd pg_isready
27+
--health-interval 10s
28+
--health-timeout 5s
29+
--health-retries 5
30+
ports:
31+
- 5432:5432
32+
33+
redis:
34+
image: redis:7
35+
options: >-
36+
--health-cmd "redis-cli ping"
37+
--health-interval 10s
38+
--health-timeout 5s
39+
--health-retries 5
40+
ports:
41+
- 6379:6379
42+
43+
env:
44+
DB_HOST: localhost
45+
DB_PORT: 5432
46+
DB_USERNAME: devtable
47+
DB_PASSWORD: devtable
48+
DB_DATABASE: devtable_test
49+
INTEGRATION_TEST_PG_URL: postgresql://devtable:devtable@localhost:5432/devtable_test_integration
50+
END_2_END_TEST_PG_URL: postgresql://devtable:devtable@localhost:5432/devtable_test_e2e
51+
REDIS_URL: redis://localhost:6379
52+
SECRET_KEY: test-secret-key-for-ci
53+
NODE_ENV: test
54+
55+
steps:
56+
- uses: actions/checkout@v4
57+
with:
58+
fetch-depth: 0
59+
60+
- name: Use Node.js
61+
uses: actions/setup-node@v4
62+
with:
63+
node-version: '20'
64+
cache: 'yarn'
65+
66+
- name: Cache Nx
67+
uses: actions/cache@v4
68+
with:
69+
path: .nx
70+
key: ${{ runner.os }}-nx-${{ hashFiles('**/yarn.lock') }}
71+
restore-keys: |
72+
${{ runner.os }}-nx-
73+
74+
- name: Install dependencies
75+
run: yarn install --frozen-lockfile
76+
77+
- name: Create test databases
78+
run: |
79+
PGPASSWORD=devtable psql -h localhost -U devtable -d devtable_test -c "CREATE DATABASE devtable_test_integration;"
80+
PGPASSWORD=devtable psql -h localhost -U devtable -d devtable_test -c "CREATE DATABASE devtable_test_e2e;"
81+
82+
- uses: nrwl/nx-set-shas@v3
83+
84+
- name: Run API unit tests
85+
working-directory: ./api
86+
run: yarn test:u
87+
88+
- name: Run API integration tests
89+
working-directory: ./api
90+
run: yarn test:i
91+
92+
- name: Run API e2e tests
93+
working-directory: ./api
94+
run: yarn test:e2e
95+
96+
- name: Upload coverage reports
97+
uses: codecov/codecov-action@v4
98+
with:
99+
files: ./api/coverage/*/lcov.info
100+
flags: api
101+
fail_ci_if_error: false

‎.gitignore‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,5 @@ api/.jest_cache
4242
api/coverage*
4343
api/yarn.lock
4444
.nx/cache
45-
.nx/workspace-data
45+
.nx/workspace-data
46+
.vscode/

‎.vscode/settings.json‎

Lines changed: 0 additions & 12 deletions
This file was deleted.

‎api/src/services/datasource.service.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,12 @@ export class DataSourceService {
148148
const source = new Source(configuration);
149149
try {
150150
await source.initialize();
151+
await source.destroy();
151152
} catch (error) {
152-
const message = error.message ?? translate('DATASOURCE_CONNECTION_TEST_FAILED', locale);
153-
throw new ApiError(BAD_REQUEST, { message });
153+
// Log the original error for debugging purposes
154+
console.error('Database connection test failed:', error.message);
155+
// Always use the generic error message to avoid exposing internal connection details
156+
throw new ApiError(BAD_REQUEST, { message: translate('DATASOURCE_CONNECTION_TEST_FAILED', locale) });
154157
}
155-
await source.destroy();
156158
}
157159
}

‎api/src/services/job.service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ export class JobService {
228228
}
229229
jobs = await jobRepo
230230
.createQueryBuilder('job')
231-
.where('type = :type', { type: JobType.RENAME_DATASOURCE })
231+
.where('type = :type', { type: JobType.FIX_DASHBOARD_PERMISSION })
232232
.andWhere('status = :status', { status: JobStatus.INIT })
233233
.orderBy('create_time', 'ASC')
234234
.getMany();

‎api/tests/e2e/06_query.test.ts‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,38 +48,38 @@ describe('QueryController', () => {
4848
id: 'pgQuery',
4949
type: 'postgresql',
5050
key: 'preset',
51-
sql: 'SELECT ${sql_snippets.role_columns} FROM role WHERE id = ${filters.role_id} AND ${context.true}',
51+
config: { _type: 'postgresql', sql: 'SELECT ${sql_snippets.role_columns} FROM role WHERE id = ${filters.role_id} AND ${context.true}' },
5252
pre_process: '',
5353
},
5454
{
5555
id: 'httpGetQuery',
5656
type: 'http',
5757
key: 'jsonplaceholder_renamed',
58-
sql: '',
58+
config: { _type: 'http', react_to: [] },
5959
pre_process:
6060
'function build_request({ context, filters }, utils) {\n const data = {};\n const headers = { "Content-Type": "application/json" };\n\n return {\n method: "GET",\n url: "/posts/1",\n params: {},\n headers,\n data,\n };\n}\n',
6161
},
6262
{
6363
id: 'httpPostQuery',
6464
type: 'http',
6565
key: 'jsonplaceholder_renamed',
66-
sql: '',
66+
config: { _type: 'http', react_to: [] },
6767
pre_process:
6868
'function build_request({ context, filters }, utils) {\n const data = { "title": "foo", "body": "bar", "userId": 1 };\n const headers = { "Content-Type": "application/json" };\n\n return {\n method: "POST",\n url: "/posts",\n params: {},\n headers,\n data,\n };\n}\n',
6969
},
7070
{
7171
id: 'httpPutQuery',
7272
type: 'http',
7373
key: 'jsonplaceholder_renamed',
74-
sql: '',
74+
config: { _type: 'http', react_to: [] },
7575
pre_process:
7676
'function build_request({ context, filters }, utils) {\n const data = { "id": 1, "title": "foo", "body": "bar", "userId": 1 };\n const headers = { "Content-Type": "application/json" };\n\n return {\n method: "PUT",\n url: "/posts/1",\n params: {},\n headers,\n data,\n };\n}\n',
7777
},
7878
{
7979
id: 'httpDeleteQuery',
8080
type: 'http',
8181
key: 'jsonplaceholder_renamed',
82-
sql: '',
82+
config: { _type: 'http', react_to: [] },
8383
pre_process:
8484
'function build_request({ context, filters }, utils) {\n const data = {};\n const headers = { "Content-Type": "application/json" };\n\n return {\n method: "DELETE",\n url: "/posts/1",\n params: {},\n headers,\n data,\n };\n}\n',
8585
},
@@ -238,13 +238,10 @@ describe('QueryController', () => {
238238
.post('/query/structure')
239239
.set('Authorization', `Bearer ${superadminLogin.token}`)
240240
.send(query);
241-
242-
expect(response.body.length).toEqual(222);
243-
expect(response.body[212]).toMatchObject({
244-
table_schema: 'public',
245-
table_name: 'dashboard',
246-
table_type: 'BASE TABLE',
247-
});
241+
// Filter to only public schema tables to avoid version-specific system table counts
242+
const publicTables = response.body.filter((t: any) => t.table_schema === 'public');
243+
expect(publicTables.length).toEqual(14);
244+
expect(publicTables).toMatchSnapshot();
248245
});
249246

250247
it('query_type = COLUMNS', async () => {

‎api/tests/e2e/__snapshots__/06_query.test.ts.snap‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`QueryController queryStructure query_type = TABLES 1`] = `
4+
[
5+
{
6+
"table_name": "account",
7+
"table_schema": "public",
8+
"table_type": "BASE TABLE",
9+
},
10+
{
11+
"table_name": "api_key",
12+
"table_schema": "public",
13+
"table_type": "BASE TABLE",
14+
},
15+
{
16+
"table_name": "config",
17+
"table_schema": "public",
18+
"table_type": "BASE TABLE",
19+
},
20+
{
21+
"table_name": "custom_function",
22+
"table_schema": "public",
23+
"table_type": "BASE TABLE",
24+
},
25+
{
26+
"table_name": "dashboard",
27+
"table_schema": "public",
28+
"table_type": "BASE TABLE",
29+
},
30+
{
31+
"table_name": "dashboard_changelog",
32+
"table_schema": "public",
33+
"table_type": "BASE TABLE",
34+
},
35+
{
36+
"table_name": "dashboard_content",
37+
"table_schema": "public",
38+
"table_type": "BASE TABLE",
39+
},
40+
{
41+
"table_name": "dashboard_content_changelog",
42+
"table_schema": "public",
43+
"table_type": "BASE TABLE",
44+
},
45+
{
46+
"table_name": "dashboard_permission",
47+
"table_schema": "public",
48+
"table_type": "BASE TABLE",
49+
},
50+
{
51+
"table_name": "data_source",
52+
"table_schema": "public",
53+
"table_type": "BASE TABLE",
54+
},
55+
{
56+
"table_name": "job",
57+
"table_schema": "public",
58+
"table_type": "BASE TABLE",
59+
},
60+
{
61+
"table_name": "role",
62+
"table_schema": "public",
63+
"table_type": "BASE TABLE",
64+
},
65+
{
66+
"table_name": "schema_migrations",
67+
"table_schema": "public",
68+
"table_type": "BASE TABLE",
69+
},
70+
{
71+
"table_name": "sql_snippet",
72+
"table_schema": "public",
73+
"table_type": "BASE TABLE",
74+
},
75+
]
76+
`;

‎api/tests/integration/06_query.test.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,9 @@ describe('QueryService', () => {
127127
describe('queryStructure', () => {
128128
it('TABLES', async () => {
129129
const results = await queryService.queryStructure('TABLES', 'postgresql', 'pg', '', '');
130-
expect(results.length).toEqual(222);
131-
expect(results[212]).toMatchObject({
132-
table_schema: 'public',
133-
table_name: 'dashboard',
134-
table_type: 'BASE TABLE',
135-
});
130+
const tablesInPublicSchema = results.filter((result) => result.table_schema === 'public');
131+
expect(tablesInPublicSchema.length).toEqual(14);
132+
expect(tablesInPublicSchema).toMatchSnapshot();
136133
});
137134

138135
it('COLUMNS', async () => {

‎api/tests/integration/12_dashboard_content_changelog.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ describe('DashboardContentChangelogService', () => {
2323
expect(results.data[0].diff).toContain('--- a/data.json\n' + '+++ b/data.json');
2424
expect(results.data[0].diff).toContain('@@ -8,7 +8,7 @@');
2525
expect(results.data[0].diff).toContain('-\t\t\t\t\t"key": "pg",\n' + '+\t\t\t\t\t"key": "pg_renamed",\n');
26-
expect(results.data[0].diff).toContain('@@ -49,5 +49,9 @@');
26+
expect(results.data[0].diff).toContain('@@ -64,5 +64,9 @@');
2727
expect(results.data[0].diff).toContain(
2828
'-\t}\n' +
2929
'+\t},\n' +
@@ -38,7 +38,7 @@ describe('DashboardContentChangelogService', () => {
3838
expect(results.data[1].diff).toContain('--- a/data.json\n' + '+++ b/data.json');
3939
expect(results.data[1].diff).toContain('@@ -8,7 +8,7 @@');
4040
expect(results.data[1].diff).toContain('-\t\t\t\t\t"key": "pg",\n' + '+\t\t\t\t\t"key": "pg_renamed",\n');
41-
expect(results.data[1].diff).toContain('@@ -23,5 +23,9 @@');
41+
expect(results.data[1].diff).toContain('@@ -29,5 +29,9 @@');
4242
expect(results.data[1].diff).toContain(
4343
'-\t}\n' +
4444
'+\t},\n' +
@@ -51,14 +51,14 @@ describe('DashboardContentChangelogService', () => {
5151

5252
expect(results.data[2].diff).toContain('diff --git a/data.json b/data.json');
5353
expect(results.data[2].diff).toContain('--- a/data.json\n' + '+++ b/data.json');
54-
expect(results.data[2].diff).toContain('@@ -15,28 +15,28 @@');
54+
expect(results.data[2].diff).toContain('@@ -18,7 +18,7 @@');
5555
expect(results.data[2].diff).toContain(
5656
'-\t\t\t\t\t"key": "jsonplaceholder",\n' + '+\t\t\t\t\t"key": "jsonplaceholder_renamed",\n',
5757
);
5858

5959
expect(results.data[3].diff).toContain('diff --git a/data.json b/data.json');
6060
expect(results.data[3].diff).toContain('--- a/data.json\n' + '+++ b/data.json');
61-
expect(results.data[3].diff).toContain('@@ -15,7 +15,7 @@');
61+
expect(results.data[3].diff).toContain('@@ -18,7 +18,7 @@');
6262
expect(results.data[3].diff).toContain(
6363
'-\t\t\t\t\t"key": "jsonplaceholder",\n' + '+\t\t\t\t\t"key": "jsonplaceholder_renamed",\n',
6464
);
@@ -80,7 +80,7 @@ describe('DashboardContentChangelogService', () => {
8080
expect(results.data[0].diff).toContain('--- a/data.json\n' + '+++ b/data.json');
8181
expect(results.data[0].diff).toContain('@@ -8,7 +8,7 @@');
8282
expect(results.data[0].diff).toContain('-\t\t\t\t\t"key": "pg",\n' + '+\t\t\t\t\t"key": "pg_renamed",\n');
83-
expect(results.data[0].diff).toContain('@@ -49,5 +49,9 @@');
83+
expect(results.data[0].diff).toContain('@@ -64,5 +64,9 @@');
8484
expect(results.data[0].diff).toContain(
8585
'-\t}\n' +
8686
'+\t},\n' +
@@ -93,7 +93,7 @@ describe('DashboardContentChangelogService', () => {
9393

9494
expect(results.data[1].diff).toContain('diff --git a/data.json b/data.json');
9595
expect(results.data[1].diff).toContain('--- a/data.json\n' + '+++ b/data.json');
96-
expect(results.data[1].diff).toContain('@@ -15,28 +15,28 @@');
96+
expect(results.data[1].diff).toContain('@@ -18,7 +18,7 @@');
9797
expect(results.data[1].diff).toContain(
9898
'-\t\t\t\t\t"key": "jsonplaceholder",\n' + '+\t\t\t\t\t"key": "jsonplaceholder_renamed",\n',
9999
);

0 commit comments

Comments
 (0)