Improve the test results for Integrations internals (#2376)

* Add custom expect handlers for integration results

Signed-off-by: Simeon Widdis <[email protected]>

* Add context to integrationReader errors

Signed-off-by: Simeon Widdis <[email protected]>

* Use FileParams type where applicable

Signed-off-by: Simeon Widdis <[email protected]>

* Assert serialized integrations works as part of usage

Signed-off-by: Simeon Widdis <[email protected]>

---------

Signed-off-by: Simeon Widdis <[email protected]>

Author: Swiddis

.eslintrc.js

@@ -39,6 +39,13 @@ module.exports = {
         ],
       },
     ],
+    'jest/expect-expect': [
+      'warn',
+      {
+        // Allow using custom expect test helpers as long as the name starts with `expect`.
+        assertFunctionNames: ["expect*"],
+      }
+    ]
   },
   overrides: [
     {

server/adaptors/integrations/__test__/custom_expects.ts

@@ -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(),
+  });
+};

server/adaptors/integrations/__test__/json_repository.test.ts

@@ -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');
@@ -31,14 +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.error).not.toBeDefined();
-    expect(serialized.ok).toBe(true);
+    expectOkResult(serialized);
   });
 
   it('Should pass deep validation for all serialized integrations', async () => {
@@ -49,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);
     }
   });
 
@@ -61,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);
   });
 
@@ -73,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);
   });
 
@@ -95,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 () => {
@@ -108,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);
   });
 
@@ -130,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);
   });
 });
@@ -151,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 () => {
@@ -170,7 +174,7 @@ describe('Integration validation', () => {
       new JsonCatalogDataAdaptor(transformedSerialized)
     );
 
-    await expect(deepCheck(integration)).resolves.toHaveProperty('ok', false);
+    expectErrorResult(await deepCheck(integration));
   });
 });
 
@@ -197,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 () => {
@@ -218,6 +221,6 @@ describe('JSON Catalog with invalid data', () => {
       new JsonCatalogDataAdaptor([baseConfig])
     );
 
-    await expect(reader.getSchemas()).resolves.toHaveProperty('ok', false);
+    expectErrorResult(await reader.getSchemas());
   });
 });

server/adaptors/integrations/__test__/local_fs_repository.test.ts

@@ -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')),
@@ -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!);
   });
 });

server/adaptors/integrations/repository/__test__/integration_reader.test.ts

@@ -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');
 
@@ -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(
+      expect(result.error?.message).toContain(
         "Unable to parse file 'sample-2.0.0.json' as JSON or NDJson"
       );
     });
@@ -114,7 +115,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' },
@@ -136,7 +137,7 @@ describe('Integration', () => {
 
       const result = await integration.getAssets(TEST_INTEGRATION_CONFIG.version);
 
-      expect(result.error?.message).toBe(
+      expect(result.error?.message).toContain(
         "Unable to parse file 'sample-1.0.1.ndjson' as JSON or NDJson"
       );
     });
@@ -178,7 +179,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 () => {
@@ -188,7 +189,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');
     });
   });
 
@@ -200,8 +201,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'));
     });
 
@@ -247,7 +248,7 @@ describe('Integration', () => {
 
       const result = await integration.getSampleData();
 
-      expect(result.ok).toBe(true);
+      expectOkResult(result);
       expect((result as { value: { sampleData: unknown } }).value.sampleData).toBeNull();
     });
 
@@ -266,7 +267,9 @@ describe('Integration', () => {
 
       const result = await integration.getSampleData();
 
-      expect(result.error?.message).toBe("Unable to parse file 'sample.json' as JSON or NDJson");
+      expect(result.error?.message).toContain(
+        "Unable to parse file 'sample.json' as JSON or NDJson"
+      );
     });
   });
 });