Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Manual Backport] Backport 2376 to 2.x #2378

Merged
merged 1 commit into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check warning on line 1 in .eslintrc.js

View workflow job for this annotation

GitHub Actions / Lint

File ignored by default.
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -39,6 +39,13 @@
],
},
],
'jest/expect-expect': [
'warn',
{
// Allow using custom expect test helpers as long as the name starts with `expect`.
assertFunctionNames: ["expect*"],
}
]
},
overrides: [
{
Expand Down
37 changes: 37 additions & 0 deletions server/adaptors/integrations/__test__/custom_expects.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

/**
* Useful for asserting results are okay, while still having access to error information. A context
* object can be supplied to help provide context if the value of the result doesn't contain enough
* information to know what went wrong.
*/
export const expectOkResult = (result: Result<unknown>, context?: string | object) => {
const labeled = {
...result,
context,
};
expect(labeled).toEqual({
ok: true,
context,
value: expect.anything(),
});
};

/**
* Validate an error result is correctly returned. A context object can be supplied to help provide
* context if the value of the result doesn't contain enough information to know what went wrong.
*/
export const expectErrorResult = (result: Result<unknown>, context?: string | object) => {
const labeled = {
...result,
context,
};
expect(labeled).toEqual({
ok: false,
context,
error: expect.anything(),
});
};
30 changes: 17 additions & 13 deletions server/adaptors/integrations/__test__/json_repository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import path from 'path';
import * as fs from 'fs/promises';
import { JsonCatalogDataAdaptor } from '../repository/json_data_adaptor';
import { deepCheck, foldResults } from '../repository/utils';
import { expectErrorResult, expectOkResult } from './custom_expects';

const fetchSerializedIntegrations = async (): Promise<Result<SerializedIntegration[], Error>> => {
const directory = path.join(__dirname, '../__data__/repository');
Expand All @@ -31,13 +32,15 @@ const fetchSerializedIntegrations = async (): Promise<Result<SerializedIntegrati
const serializedIntegrationResults = await Promise.all(
(readers.filter((x) => x !== null) as IntegrationReader[]).map((r) => r.serialize())
);
return foldResults(serializedIntegrationResults);
const folded = foldResults(serializedIntegrationResults);
expectOkResult(folded);
return folded;
};

describe('The Local Serialized Catalog', () => {
it('Should serialize without errors', async () => {
const serialized = await fetchSerializedIntegrations();
expect(serialized.ok).toBe(true);
expectOkResult(serialized);
});

it('Should pass deep validation for all serialized integrations', async () => {
Expand All @@ -48,7 +51,7 @@ describe('The Local Serialized Catalog', () => {

for (const integ of await repository.getIntegrationList()) {
const validationResult = await deepCheck(integ);
await expect(validationResult).toHaveProperty('ok', true);
expectOkResult(validationResult);
}
});

Expand All @@ -60,7 +63,7 @@ describe('The Local Serialized Catalog', () => {
const integration = (await repository.getIntegration('nginx')) as IntegrationReader;
const logoStatic = await integration.getStatic('logo.svg');

expect(logoStatic).toHaveProperty('ok', true);
expectOkResult(logoStatic);
expect((logoStatic.value as Buffer).length).toBeGreaterThan(100);
});

Expand All @@ -72,7 +75,7 @@ describe('The Local Serialized Catalog', () => {
const integration = (await repository.getIntegration('nginx')) as IntegrationReader;
const logoStatic = await integration.getStatic('dashboard1.png');

expect(logoStatic).toHaveProperty('ok', true);
expectOkResult(logoStatic);
expect((logoStatic.value as Buffer).length).toBeGreaterThan(1000);
});

Expand All @@ -94,7 +97,7 @@ describe('The Local Serialized Catalog', () => {

const reader = new IntegrationReader('nginx', new JsonCatalogDataAdaptor([config]));

await expect(reader.getStatic('dark_logo.svg')).resolves.toHaveProperty('ok', true);
expectOkResult(await reader.getStatic('dark_logo.svg'));
});

it('Should correctly re-serialize', async () => {
Expand All @@ -107,6 +110,7 @@ describe('The Local Serialized Catalog', () => {
const reader = new IntegrationReader('nginx', new JsonCatalogDataAdaptor([config]));
const reserialized = await reader.serialize();

expectOkResult(reserialized);
expect(reserialized.value).toEqual(config);
});

Expand All @@ -129,6 +133,7 @@ describe('The Local Serialized Catalog', () => {
const reader = new IntegrationReader('nginx', new JsonCatalogDataAdaptor([config]));
const reserialized = await reader.serialize();

expectOkResult(reserialized);
expect(reserialized.value).toEqual(config);
});
});
Expand All @@ -150,7 +155,7 @@ describe('Integration validation', () => {
new JsonCatalogDataAdaptor(transformedSerialized)
);

await expect(deepCheck(integration)).resolves.toHaveProperty('ok', false);
expectErrorResult(await deepCheck(integration));
});

it('Should correctly fail an integration without assets', async () => {
Expand All @@ -169,7 +174,7 @@ describe('Integration validation', () => {
new JsonCatalogDataAdaptor(transformedSerialized)
);

await expect(deepCheck(integration)).resolves.toHaveProperty('ok', false);
expectErrorResult(await deepCheck(integration));
});
});

Expand All @@ -196,10 +201,9 @@ describe('JSON Catalog with invalid data', () => {
new JsonCatalogDataAdaptor([baseConfig])
);

await expect(reader.getStatic('logo.svg')).resolves.toHaveProperty('ok', false);
await expect(reader.getStatic('dm_logo.svg')).resolves.toHaveProperty('ok', false);
await expect(reader.getStatic('1.png')).resolves.toHaveProperty('ok', false);
await expect(reader.getStatic('dm_1.png')).resolves.toHaveProperty('ok', false);
for (const img of ['logo.svg', 'dm_logo.svg', '1.png', 'dm_1.png']) {
expectErrorResult(await reader.getStatic(img));
}
});

it('Should report an error on read if a schema has invalid JSON', async () => {
Expand All @@ -217,6 +221,6 @@ describe('JSON Catalog with invalid data', () => {
new JsonCatalogDataAdaptor([baseConfig])
);

await expect(reader.getSchemas()).resolves.toHaveProperty('ok', false);
expectErrorResult(await reader.getSchemas());
});
});
44 changes: 31 additions & 13 deletions server/adaptors/integrations/__test__/local_fs_repository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import path from 'path';
import * as fs from 'fs/promises';
import { deepCheck } from '../repository/utils';
import { FileSystemDataAdaptor } from '../repository/fs_data_adaptor';
import { expectOkResult } from './custom_expects';

const repository: TemplateManager = new TemplateManager([
new FileSystemDataAdaptor(path.join(__dirname, '../__data__/repository')),
Expand All @@ -31,40 +32,57 @@ describe('The local repository', () => {
}
// Otherwise, all directories must be integrations
const integ = new IntegrationReader(integPath);
await expect(integ.getConfig()).resolves.toMatchObject({ ok: true });
const config = await integ.getConfig();
expectOkResult(config, { integration: integ.name });
})
);
});

it('Should pass deep validation for all local integrations.', async () => {
const integrations: IntegrationReader[] = await repository.getIntegrationList();
await Promise.all(
integrations.map(async (i: IntegrationReader) => {
const result = await deepCheck(i);
if (!result.ok) {
console.error(i.directory, result.error);
}
expect(result.ok).toBe(true);
integrations.map(async (integ: IntegrationReader) => {
const result = await deepCheck(integ);
expectOkResult(result, { integration: integ.name });
})
);
});
});

// Nginx and VPC are specifically used in other tests, so we add dedicated checks for them.

describe('Local Nginx Integration', () => {
it('Should serialize without errors', async () => {
const integration = await repository.getIntegration('nginx');

await expect(integration?.serialize()).resolves.toHaveProperty('ok', true);
expect(integration).not.toBeNull();
expectOkResult(await integration!.serialize());
});

it('Should serialize to include the config', async () => {
it('Should contain its config in its serialized form', async () => {
const integration = await repository.getIntegration('nginx');
const config = await integration!.getConfig();
const serialized = await integration!.serialize();

expect(serialized).toHaveProperty('ok', true);
expect((serialized as { value: object }).value).toMatchObject(
(config as { value: object }).value
);
expectOkResult(serialized);
expect(serialized.value).toMatchObject(config.value!);
});
});

describe('Local VPC Integration', () => {
it('Should serialize without errors', async () => {
const integration = await repository.getIntegration('amazon_vpc_flow');

expect(integration).not.toBeNull();
expectOkResult(await integration!.serialize());
});

it('Should contain its config in its serialized form', async () => {
const integration = await repository.getIntegration('amazon_vpc_flow');
const config = await integration!.getConfig();
const serialized = await integration!.serialize();

expectOkResult(serialized);
expect(serialized.value).toMatchObject(config.value!);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { IntegrationReader } from '../integration_reader';
import { Dirent, Stats } from 'fs';
import * as path from 'path';
import { TEST_INTEGRATION_CONFIG } from '../../../../../test/constants';
import { expectOkResult } from '../../__test__/custom_expects';

jest.mock('fs/promises');

Expand Down Expand Up @@ -75,15 +76,15 @@ describe('Integration', () => {

const result = await integration.getConfig(TEST_INTEGRATION_CONFIG.version);

expect(result.error?.message).toBe('data/version must be string');
expect(result.error?.message).toContain('data/version must be string');
});

it('should return an error if the config file has syntax errors', async () => {
jest.spyOn(fs, 'readFile').mockResolvedValue('Invalid JSON');

const result = await integration.getConfig(TEST_INTEGRATION_CONFIG.version);

expect(result.error?.message).toBe('Unable to parse file as JSON or NDJson');
expect(result.error?.message).toContain('Unable to parse file as JSON or NDJson');
});

it('should return an error if the integration config does not exist', async () => {
Expand Down Expand Up @@ -112,7 +113,7 @@ describe('Integration', () => {

const result = await integration.getAssets(TEST_INTEGRATION_CONFIG.version);

expect(result.ok).toBe(true);
expectOkResult(result);
expect((result as { value: Array<{ data: object[] }> }).value[0].data).toEqual([
{ name: 'asset1' },
{ name: 'asset2' },
Expand All @@ -134,7 +135,7 @@ describe('Integration', () => {

const result = await integration.getAssets(TEST_INTEGRATION_CONFIG.version);

expect(result.error?.message).toBe('Unable to parse file as JSON or NDJson');
expect(result.error?.message).toContain('Unable to parse file as JSON or NDJson');
});
});

Expand Down Expand Up @@ -174,7 +175,7 @@ describe('Integration', () => {
);
const result = await integration.getSchemas();

expect(result.error?.message).toBe("data must have required property 'name'");
expect(result.error?.message).toContain("data must have required property 'name'");
});

it('should reject with an error if a mapping file is invalid', async () => {
Expand All @@ -184,7 +185,7 @@ describe('Integration', () => {
.mockRejectedValueOnce(new Error('Could not load schema'));

const result = await integration.getSchemas();
expect(result.error?.message).toBe('Could not load schema');
expect(result.error?.message).toContain('Could not load schema');
});
});

Expand All @@ -196,8 +197,8 @@ describe('Integration', () => {

const result = await integration.getStatic('logo.png');

expect(result.ok).toBe(true);
expect((result as { value: unknown }).value).toStrictEqual(Buffer.from('logo data', 'ascii'));
expectOkResult(result);
expect(result.value).toStrictEqual(Buffer.from('logo data', 'ascii'));
expect(readFileMock).toBeCalledWith(path.join('sample', 'static', 'logo.png'));
});

Expand Down Expand Up @@ -243,7 +244,7 @@ describe('Integration', () => {

const result = await integration.getSampleData();

expect(result.ok).toBe(true);
expectOkResult(result);
expect((result as { value: { sampleData: unknown } }).value.sampleData).toBeNull();
});

Expand All @@ -262,7 +263,7 @@ describe('Integration', () => {

const result = await integration.getSampleData();

expect(result.error?.message).toBe('Unable to parse file as JSON or NDJson');
expect(result.error?.message).toContain('Unable to parse file as JSON or NDJson');
});
});
});
Loading
Loading