Skip to content

Commit ff9fc0d

Browse files
kravets-levkokravets-levko
authored
Re-enable staging ingestion e2e tests (#221)
* Re-enable staging ingestion e2e tests Signed-off-by: Levko Kravets <[email protected]> * Update volume ingestion e2e tests Signed-off-by: Levko Kravets <[email protected]> * Update tests Signed-off-by: Levko Kravets <[email protected]> --------- Signed-off-by: Levko Kravets <[email protected]>
1 parent 340a63a commit ff9fc0d

File tree

5 files changed

+102
-46
lines changed

5 files changed

+102
-46
lines changed

‎.github/workflows/main.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ jobs:
7474
E2E_PATH: ${{ secrets.TEST_PECO_WAREHOUSE_HTTP_PATH }}
7575
E2E_ACCESS_TOKEN: ${{ secrets.DATABRICKS_TOKEN }}
7676
E2E_TABLE_SUFFIX: ${{github.sha}}
77+
E2E_CATALOG: peco
78+
E2E_SCHEMA: default
79+
E2E_VOLUME: e2etests
7780
cache-name: cache-node-modules
7881
NYC_REPORT_DIR: coverage_e2e
7982

‎lib/DBSQLSession.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,10 @@ export default class DBSQLSession implements IDBSQLSession {
280280
const agent = await connectionProvider.getAgent();
281281

282282
const response = await fetch(presignedUrl, { method: 'DELETE', headers, agent });
283+
// Looks that AWS and Azure have a different behavior of HTTP `DELETE` for non-existing files
284+
// AWS assumes that - since file already doesn't exist - the goal is achieved, and returns HTTP 200
285+
// Azure, on the other hand, is somewhat stricter and check if file exists before deleting it. And if
286+
// file doesn't exist - Azure returns HTTP 404
283287
if (!response.ok) {
284288
throw new StagingError(`HTTP error ${response.status} ${response.statusText}`);
285289
}

‎tests/e2e/staging/.gitignore‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎tests/e2e/staging_ingestion.test.js‎

Lines changed: 91 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,61 @@
11
const { expect } = require('chai');
2+
const fs = require('fs');
3+
const path = require('path');
4+
const os = require('os');
5+
const uuid = require('uuid');
26
const config = require('./utils/config');
37
const { DBSQLClient } = require('../..');
4-
const fs = require('fs');
8+
const StagingError = require('../../dist/errors/StagingError').default;
9+
10+
describe('Staging Test', () => {
11+
const catalog = config.database[0];
12+
const schema = config.database[1];
13+
const volume = config.volume;
14+
15+
const localPath = fs.mkdtempSync(path.join(os.tmpdir(), 'databricks-sql-tests-'));
16+
17+
before(() => {
18+
expect(catalog).to.not.be.undefined;
19+
expect(schema).to.not.be.undefined;
20+
expect(volume).to.not.be.undefined;
21+
});
22+
23+
after(() => {
24+
fs.rmSync(localPath, {
25+
recursive: true,
26+
force: true,
27+
});
28+
});
529

6-
// TODO: Temporarily disable those tests until we figure out issues with E2E test env
7-
describe.skip('Staging Test', () => {
830
it('put staging data and receive it', async () => {
931
const client = new DBSQLClient();
1032
await client.connect({
1133
host: config.host,
1234
path: config.path,
1335
token: config.token,
1436
});
15-
let tempPath = 'tests/e2e/staging/data';
16-
fs.writeFileSync(tempPath, 'Hello World!');
1737

1838
const session = await client.openSession({
19-
initialCatalog: config.database[0],
20-
initialSchema: config.database[1],
39+
initialCatalog: catalog,
40+
initialSchema: schema,
41+
});
42+
43+
const expectedData = 'Hello World!';
44+
const stagingFileName = `/Volumes/${catalog}/${schema}/${volume}/${uuid.v4()}.csv`;
45+
const localFile = path.join(localPath, `${uuid.v4()}.csv`);
46+
47+
fs.writeFileSync(localFile, expectedData);
48+
await session.executeStatement(`PUT '${localFile}' INTO '${stagingFileName}' OVERWRITE`, {
49+
stagingAllowedLocalPath: [localPath],
50+
});
51+
fs.rmSync(localFile);
52+
53+
await session.executeStatement(`GET '${stagingFileName}' TO '${localFile}'`, {
54+
stagingAllowedLocalPath: [localPath],
2155
});
22-
await session.executeStatement(
23-
`PUT '${tempPath}' INTO '/Volumes/${config.database[0]}/${config.database[1]}/e2etests/file1.csv' OVERWRITE`,
24-
{ stagingAllowedLocalPath: ['tests/e2e/staging'] },
25-
);
26-
await session.executeStatement(
27-
`GET '/Volumes/${config.database[0]}/${config.database[1]}/e2etests/file1.csv' TO 'tests/e2e/staging/file'`,
28-
{ stagingAllowedLocalPath: ['tests/e2e/staging'] },
29-
);
30-
let result = fs.readFileSync('tests/e2e/staging/file');
31-
expect(result.toString() === 'Hello World!').to.be.true;
56+
const result = fs.readFileSync(localFile);
57+
fs.rmSync(localFile);
58+
expect(result.toString() === expectedData).to.be.true;
3259
});
3360

3461
it('put staging data and remove it', async () => {
@@ -38,20 +65,39 @@ describe.skip('Staging Test', () => {
3865
path: config.path,
3966
token: config.token,
4067
});
41-
let tempPath = 'tests/e2e/staging/data';
42-
fs.writeFileSync(tempPath, (data = 'Hello World!'));
4368

44-
let session = await client.openSession({
45-
initialCatalog: config.database[0],
46-
initialSchema: config.database[1],
69+
const session = await client.openSession({
70+
initialCatalog: catalog,
71+
initialSchema: schema,
4772
});
48-
await session.executeStatement(
49-
`PUT '${tempPath}' INTO '/Volumes/${config.database[0]}/${config.database[1]}/e2etests/file1.csv' OVERWRITE`,
50-
{ stagingAllowedLocalPath: ['tests/e2e/staging'] },
51-
);
52-
await session.executeStatement(`REMOVE '/Volumes/${config.database[0]}/${config.database[1]}/e2etests/file1.csv'`, {
53-
stagingAllowedLocalPath: ['tests/e2e/staging'],
73+
74+
const expectedData = 'Hello World!';
75+
const stagingFileName = `/Volumes/${catalog}/${schema}/${volume}/${uuid.v4()}.csv`;
76+
const localFile = path.join(localPath, `${uuid.v4()}.csv`);
77+
78+
fs.writeFileSync(localFile, expectedData);
79+
await session.executeStatement(`PUT '${localFile}' INTO '${stagingFileName}' OVERWRITE`, {
80+
stagingAllowedLocalPath: [localPath],
5481
});
82+
fs.rmSync(localFile);
83+
84+
await session.executeStatement(`REMOVE '${stagingFileName}'`, { stagingAllowedLocalPath: [localPath] });
85+
86+
try {
87+
await session.executeStatement(`GET '${stagingFileName}' TO '${localFile}'`, {
88+
stagingAllowedLocalPath: [localPath],
89+
});
90+
expect.fail('It should throw HTTP 404 error');
91+
} catch (error) {
92+
if (error instanceof StagingError) {
93+
// File should not exist after deleting
94+
expect(error.message).to.contain('404');
95+
} else {
96+
throw error;
97+
}
98+
} finally {
99+
fs.rmSync(localFile, { force: true });
100+
}
55101
});
56102

57103
it('delete non-existent data', async () => {
@@ -61,22 +107,24 @@ describe.skip('Staging Test', () => {
61107
path: config.path,
62108
token: config.token,
63109
});
64-
let tempPath = 'tests/e2e/staging/data';
65-
fs.writeFileSync(tempPath, (data = 'Hello World!'));
66110

67-
let session = await client.openSession({
68-
initialCatalog: config.database[0],
69-
initialSchema: config.database[1],
111+
const session = await client.openSession({
112+
initialCatalog: catalog,
113+
initialSchema: schema,
70114
});
71-
await session.executeStatement(
72-
`PUT '${tempPath}' INTO '/Volumes/${config.database[0]}/${config.database[1]}/e2etests/file1.csv' OVERWRITE`,
73-
{ stagingAllowedLocalPath: ['tests/e2e/staging'] },
74-
);
75-
await session.executeStatement(
76-
`GET '/Volumes/${config.database[0]}/${config.database[1]}/e2etests/file1.csv' TO 'tests/e2e/staging/file'`,
77-
{ stagingAllowedLocalPath: ['tests/e2e/staging'] },
78-
);
79-
let result = fs.readFileSync('tests/e2e/staging/file');
80-
expect(result.toString() === 'Hello World!').to.be.true;
115+
116+
const stagingFileName = `/Volumes/${catalog}/${schema}/${volume}/${uuid.v4()}.csv`;
117+
118+
try {
119+
await session.executeStatement(`REMOVE '${stagingFileName}'`, { stagingAllowedLocalPath: [localPath] });
120+
// In some cases, `REMOVE` may silently succeed for non-existing files (see comment in relevant
121+
// part of `DBSQLSession` code). But if it fails - it has to be an HTTP 404 error
122+
} catch (error) {
123+
if (error instanceof StagingError) {
124+
expect(error.message).to.contain('404');
125+
} else {
126+
throw error;
127+
}
128+
}
81129
});
82130
});

‎tests/e2e/utils/config.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ try {
44
} catch (e) {}
55

66
const catalog = process.env.E2E_CATALOG || undefined;
7-
const database = process.env.E2E_DATABASE || undefined;
7+
const schema = process.env.E2E_SCHEMA || undefined;
88

99
// Create file named `config.local.js` in the same directory and override config there
1010
module.exports = {
@@ -17,7 +17,9 @@ module.exports = {
1717
// Access token: dapi********************************
1818
token: process.env.E2E_ACCESS_TOKEN,
1919
// Catalog and database to use for testing; specify both or leave array empty to use defaults
20-
database: catalog || database ? [catalog, database] : [],
20+
database: catalog || schema ? [catalog, schema] : [],
21+
// Volume to use for testing
22+
volume: process.env.E2E_VOLUME,
2123
// Suffix used for tables that will be created during tests
2224
tableSuffix: process.env.E2E_TABLE_SUFFIX,
2325
...overrides,

0 commit comments

Comments
 (0)