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.

Sign up for GitHub

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

Jump to bottom

[NO-TICKET] - Converting schema validation errors to warnings #869

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[NO-TICKET] - Converting schema validation errors to warnings #869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9e1ae2
Converting schema validation errors to warnings.
adam-in-ict Apr 7, 2023
5362424
Disabling eslint warning until we determine if we want to add more co…
adam-in-ict Apr 7, 2023
ae0d8c8
Updating tests that previously expected throws.
adam-in-ict Apr 7, 2023
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
25 changes: 6 additions & 19 deletions packages/integration-sdk-cli/src/__tests__/cli-run.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ test('disables graph object schema validation', async () => {
expect(process.env.ENABLE_GRAPH_OBJECT_SCHEMA_VALIDATION).toBeUndefined();
});

test('step should fail if enableSchemaValidation = true', async () => {
test('step should warn if enableSchemaValidation = true', async () => {
const consoleSpy = jest.spyOn(console, 'warn');
loadProjectStructure('instanceWithNonValidatingSteps');
const job = generateSynchronizationJob();

Expand All @@ -101,26 +102,11 @@ test('step should fail if enableSchemaValidation = true', async () => {
expect(log.displaySynchronizationResults).toHaveBeenCalledTimes(1);

expect(log.displayExecutionResults).toHaveBeenCalledTimes(1);
expect(log.displayExecutionResults).toHaveBeenCalledWith({
integrationStepResults: [
{
id: 'fetch-users',
name: 'Fetch Users',
declaredTypes: ['my_user'],
partialTypes: [],
encounteredTypes: [],
status: StepResultStatus.FAILURE,
},
],
metadata: {
partialDatasets: {
types: ['my_user'],
},
},
});
expect(consoleSpy).toHaveBeenCalled();
});

test('step should pass if enableSchemaValidation = false', async () => {
test('step should pass and not warn if enableSchemaValidation = false', async () => {
const consoleSpy = jest.spyOn(console, 'warn');
loadProjectStructure('instanceWithNonValidatingSteps');
const job = generateSynchronizationJob();

Expand Down Expand Up @@ -159,6 +145,7 @@ test('step should pass if enableSchemaValidation = false', async () => {
},
},
});
expect(consoleSpy).not.toHaveBeenCalled();
});

test('executes integration and performs upload', async () => {
Expand Down
27 changes: 6 additions & 21 deletions packages/integration-sdk-cli/src/__tests__/cli.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,30 +199,14 @@ describe('collect', () => {
delete process.env.ENABLE_GRAPH_OBJECT_SCHEMA_VALIDATION;
});

test('step should fail if enableSchemaValidation = true', async () => {
test('step should warn if enableSchemaValidation = true', async () => {
const consoleSpy = jest.spyOn(console, 'warn');
await createCli().parseAsync(['node', 'j1-integration', 'collect']);

expect(log.displayExecutionResults).toHaveBeenCalledTimes(1);
expect(log.displayExecutionResults).toHaveBeenCalledWith({
integrationStepResults: [
{
id: 'fetch-users',
name: 'Fetch Users',
declaredTypes: ['my_user'],
partialTypes: [],
encounteredTypes: [],
status: StepResultStatus.FAILURE,
},
],
metadata: {
partialDatasets: {
types: ['my_user'],
},
},
});
expect(consoleSpy).toHaveBeenCalled();
});

test('step should pass if enableSchemaValidation = false', async () => {
test('step should pass and not warn if enableSchemaValidation = false', async () => {
const consoleSpy = jest.spyOn(console, 'warn');
await createCli().parseAsync([
'node',
'j1-integration',
Expand All @@ -248,6 +232,7 @@ describe('collect', () => {
},
},
});
expect(consoleSpy).not.toHaveBeenCalled();
});
});

Expand Down
10 changes: 7 additions & 3 deletions packages/integration-sdk-core/src/data/__tests__/createIntegrationEntity.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,8 @@ describe('schema validation on', () => {
delete process.env.ENABLE_GRAPH_OBJECT_SCHEMA_VALIDATION;
});

test('throws if an a required property is not set', () => {
test('warns if an a required property is not set', () => {
const consoleSpy = jest.spyOn(console, 'warn');
expect(() =>
createIntegrationEntity({
entityData: {
Expand All @@ -340,7 +341,8 @@ describe('schema validation on', () => {
},
},
}),
).toThrow(/required property 'name'/);
).not.toThrow();
expect(consoleSpy).toHaveBeenCalled();
});
});

Expand All @@ -349,7 +351,8 @@ describe('schema validation off', () => {
delete process.env.ENABLE_GRAPH_OBJECT_SCHEMA_VALIDATION;
});

test('does not throw if class is not in data model', () => {
test('does not warn if class is not in data model', () => {
const consoleSpy = jest.spyOn(console, 'warn');
expect(() =>
createIntegrationEntity({
entityData: {
Expand All @@ -361,5 +364,6 @@ describe('schema validation off', () => {
},
}),
).not.toThrow();
expect(consoleSpy).not.toHaveBeenCalled();
});
});
7 changes: 6 additions & 1 deletion packages/integration-sdk-core/src/data/createIntegrationEntity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,12 @@ export function createIntegrationEntity(
validateRawData(generatedEntity);

if (process.env.ENABLE_GRAPH_OBJECT_SCHEMA_VALIDATION) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should remove support for the env var.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meaning, during development we should always being logging the warnings but still not in production.

validateEntityWithSchema(generatedEntity);
try {
validateEntityWithSchema(generatedEntity);
} catch (err) {
/* eslint-disable no-console */
console.warn(err);
}
}

return generatedEntity;
Expand Down