1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-01-20 00:08:02 +01:00

fix: add truth checks to a number of jest tests. (#3627)

This commit is contained in:
Thomas Heartman 2023-04-26 14:24:09 +02:00 committed by GitHub
parent 9cdc3190bc
commit 0613ee0bd9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 132 additions and 46 deletions

View File

@ -21,9 +21,9 @@ describe('OpenAPI error conversion', () => {
const { description } = fromOpenApiValidationError({})(error); const { description } = fromOpenApiValidationError({})(error);
// it tells the user that the property is required // it tells the user that the property is required
expect(description.includes('required')); expect(description.includes('required')).toBeTruthy();
// it tells the user the name of the missing property // it tells the user the name of the missing property
expect(description.includes(error.params.missingProperty)); expect(description.includes(error.params.missingProperty)).toBeTruthy();
}); });
it('Gives useful error messages for type errors', () => { it('Gives useful error messages for type errors', () => {
@ -45,9 +45,11 @@ describe('OpenAPI error conversion', () => {
})(error); })(error);
// it provides the message // it provides the message
expect(description.includes(error.message)); expect(description.includes(error.message)).toBeTruthy();
// it tells the user what they provided // it tells the user what they provided
expect(description.includes(JSON.stringify(parameterValue))); expect(
description.includes(JSON.stringify(parameterValue)),
).toBeTruthy();
}); });
it('Gives useful pattern error messages', () => { it('Gives useful pattern error messages', () => {
@ -69,11 +71,11 @@ describe('OpenAPI error conversion', () => {
})(error); })(error);
// it tells the user what the pattern it should match is // it tells the user what the pattern it should match is
expect(description.includes(error.params.pattern)); expect(description.includes(error.params.pattern)).toBeTruthy();
// it tells the user which property it pertains to // it tells the user which property it pertains to
expect(description.includes('description')); expect(description.includes('description')).toBeTruthy();
// it tells the user what they provided // it tells the user what they provided
expect(description.includes(requestDescription)); expect(description.includes(requestDescription)).toBeTruthy();
}); });
it('Gives useful min/maxlength error messages', () => { it('Gives useful min/maxlength error messages', () => {
@ -95,11 +97,13 @@ describe('OpenAPI error conversion', () => {
})(error); })(error);
// it tells the user what the pattern it should match is // it tells the user what the pattern it should match is
expect(description.includes(error.params.limit.toString())); expect(
description.includes(error.params.limit.toString()),
).toBeTruthy();
// it tells the user which property it pertains to // it tells the user which property it pertains to
expect(description.includes('description')); expect(description.includes('description')).toBeTruthy();
// it tells the user what they provided // it tells the user what they provided
expect(description.includes(requestDescription)); expect(description.includes(requestDescription)).toBeTruthy();
}); });
it('Handles numerical min/max errors', () => { it('Handles numerical min/max errors', () => {
@ -123,13 +127,15 @@ describe('OpenAPI error conversion', () => {
})(error); })(error);
// it tells the user what the limit is // it tells the user what the limit is
expect(description.includes(error.params.limit.toString())); expect(
description.includes(error.params.limit.toString()),
).toBeTruthy();
// it tells the user what kind of comparison it performed // it tells the user what kind of comparison it performed
expect(description.includes(error.params.comparison)); expect(description.includes(error.params.comparison)).toBeTruthy();
// it tells the user which property it pertains to // it tells the user which property it pertains to
expect(description.includes('newprop')); expect(description.includes('newprop')).toBeTruthy();
// it tells the user what they provided // it tells the user what they provided
expect(description.includes(propertyValue.toString())); expect(description.includes(propertyValue.toString())).toBeTruthy();
}); });
it('Handles multiple errors', () => { it('Handles multiple errors', () => {
@ -175,6 +181,65 @@ describe('OpenAPI error conversion', () => {
); );
}); });
describe('Disallowed additional properties', () => {
it('gives useful messages for base-level properties', () => {
const openApiError = {
keyword: 'additionalProperties',
instancePath: '',
dataPath: '.body',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/additionalProperties',
params: { additionalProperty: 'bogus' },
message: 'should NOT have additional properties',
};
const error = fromOpenApiValidationError({ bogus: 5 })(
openApiError,
);
expect(
error.description.includes(
openApiError.params.additionalProperty,
),
).toBeTruthy();
expect(error.description).toMatch(/\broot\b/i);
expect(error.description).toMatch(/\badditional properties\b/i);
});
it('gives useful messages for nested properties', () => {
const request2 = {
nestedObject: {
nested2: { extraPropertyName: 'illegal property' },
},
};
const openApiError = {
keyword: 'additionalProperties',
instancePath: '',
dataPath: '.body.nestedObject.nested2',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/nestedObject/properties/nested2/additionalProperties',
params: { additionalProperty: 'extraPropertyName' },
message: 'should NOT have additional properties',
};
const error = fromOpenApiValidationError(request2)(openApiError);
expect(
error.description.includes('nestedObject.nested2'),
).toBeTruthy();
expect(
error.description.includes(
openApiError.params.additionalProperty,
),
).toBeTruthy();
expect(
error.description
.toLowerCase()
.includes('additional properties'),
).toBeTruthy();
});
});
it('Handles deeply nested properties gracefully', () => { it('Handles deeply nested properties gracefully', () => {
const error = { const error = {
keyword: 'type', keyword: 'type',
@ -191,9 +256,9 @@ describe('OpenAPI error conversion', () => {
})(error); })(error);
// it should hold the full path to the error // it should hold the full path to the error
expect(description.includes('nestedObject.a.b')); expect(description).toMatch(/\bnestedObject.a.b\b/);
// it should include the value that the user sent // it should include the value that the user sent
expect(description.includes('[]')); expect(description.includes('[]')).toBeTruthy();
}); });
it('Handles deeply nested properties on referenced schemas', () => { it('Handles deeply nested properties on referenced schemas', () => {
@ -212,8 +277,8 @@ describe('OpenAPI error conversion', () => {
})(error); })(error);
// it should hold the full path to the error // it should hold the full path to the error
expect(description.includes('nestedObject.a.b')); expect(description).toMatch(/\bnestedObject.a.b\b/);
// it should include the value that the user sent // it should include the value that the user sent
expect(description.includes(illegalValue)); expect(description.includes(illegalValue)).toBeTruthy();
}); });
}); });

View File

@ -279,8 +279,24 @@ export const fromOpenApiValidationError =
description, description,
message: description, message: description,
}; };
} else if (validationError.keyword === 'additionalProperties') {
const path =
(propertyName ? propertyName + '.' : '') +
validationError.params.additionalProperty;
const description = `The ${
propertyName ? `\`${propertyName}\`` : 'root'
} object of the request body does not allow additional properties. Your request included the \`${path}\` property.`;
return {
path,
description,
message: description,
};
} else { } else {
const youSent = JSON.stringify(requestBody[propertyName]); const input = propertyName
.split('.')
.reduce((x, prop) => x[prop], requestBody);
const youSent = JSON.stringify(input);
const description = `The .${propertyName} property ${validationError.message}. You sent ${youSent}.`; const description = `The .${propertyName} property ${validationError.message}. You sent ${youSent}.`;
return { return {
description, description,

View File

@ -372,9 +372,13 @@ export default class ExportImportService {
'Some of the context fields you are trying to import are not supported.', 'Some of the context fields you are trying to import are not supported.',
// @ts-ignore-error We know that the array contains at least one // @ts-ignore-error We know that the array contains at least one
// element here. // element here.
errors: unsupportedContextFields.map((field) => ({ details: unsupportedContextFields.map((field) => {
description: `${field.name} is not supported.`, const description = `${field.name} is not supported.`;
})), return {
description,
message: description,
};
}),
}); });
} }
} }
@ -453,9 +457,14 @@ export default class ExportImportService {
'Some of the strategies you are trying to import are not supported.', 'Some of the strategies you are trying to import are not supported.',
// @ts-ignore-error We know that the array contains at least one // @ts-ignore-error We know that the array contains at least one
// element here. // element here.
errors: unsupportedStrategies.map((strategy) => ({ details: unsupportedStrategies.map((strategy) => {
description: `${strategy.name} is not supported.`, const description = `${strategy.name} is not supported.`;
})),
return {
description,
message: description,
};
}),
}); });
} }
} }

View File

@ -673,11 +673,7 @@ test('reject import with unknown context fields', async () => {
400, 400,
); );
expect( expect(body.details[0].description).toMatch(/\bContextField1\b/);
body.errors.includes((error) =>
error.description.includes('ContextField1'),
),
);
}); });
test('reject import with unsupported strategies', async () => { test('reject import with unsupported strategies', async () => {
@ -697,11 +693,7 @@ test('reject import with unsupported strategies', async () => {
400, 400,
); );
expect( expect(body.details[0].description).toMatch(/\bcustomStrategy\b/);
body.errors.includes((error) =>
error.description.includes('customStrategy'),
),
);
}); });
test('validate import data', async () => { test('validate import data', async () => {

View File

@ -57,7 +57,7 @@ test('require a name when creating a new strategy', async () => {
['name', 'property', 'required'].every((word) => ['name', 'property', 'required'].every((word) =>
res.body.details[0].description.includes(word), res.body.details[0].description.includes(word),
), ),
); ).toBeTruthy();
}); });
}); });

View File

@ -184,8 +184,10 @@ test('Trying to add a strategy configuration to environment not connected to tog
}) })
.expect(400) .expect(400)
.expect((r) => { .expect((r) => {
expect(r.body.message.includes('environment')); expect(
expect(r.body.message.includes('project')); r.body.details[0].message.includes('environment'),
).toBeTruthy();
expect(r.body.details[0].message.includes('project')).toBeTruthy();
}); });
}); });
@ -776,12 +778,12 @@ test('Trying to patch variants on a feature toggle should trigger an OperationDe
]) ])
.expect(403) .expect(403)
.expect((res) => { .expect((res) => {
expect(res.body.message.includes('PATCH')); expect(res.body.message.includes('PATCH')).toBeTruthy();
expect( expect(
res.body.message.includes( res.body.message.includes(
'/api/admin/projects/:project/features/:feature/variants', '/api/admin/projects/:project/features/:feature/variants',
), ),
); ).toBeTruthy();
}); });
}); });

View File

@ -777,8 +777,10 @@ test('Should be denied move feature toggle to project where the user does not ha
); );
} catch (e) { } catch (e) {
expect(e.name).toContain('NoAccess'); expect(e.name).toContain('NoAccess');
expect(e.message.includes('permission')); expect(e.message.includes('permission')).toBeTruthy();
expect(e.message.includes(permissions.MOVE_FEATURE_TOGGLE)); expect(
e.message.includes(permissions.MOVE_FEATURE_TOGGLE),
).toBeTruthy();
} }
}); });

View File

@ -538,8 +538,8 @@ test('should not change project if feature toggle project does not match current
'wrong-project-id', 'wrong-project-id',
); );
} catch (err) { } catch (err) {
expect(err.message.toLowerCase().includes('permission')); expect(err.message.toLowerCase().includes('permission')).toBeTruthy();
expect(err.message.includes(MOVE_FEATURE_TOGGLE)); expect(err.message.includes(MOVE_FEATURE_TOGGLE)).toBeTruthy();
} }
}); });
@ -604,8 +604,8 @@ test('should fail if user is not authorized', async () => {
project.id, project.id,
); );
} catch (err) { } catch (err) {
expect(err.message.toLowerCase().includes('permission')); expect(err.message.toLowerCase().includes('permission')).toBeTruthy();
expect(err.message.includes(MOVE_FEATURE_TOGGLE)); expect(err.message.includes(MOVE_FEATURE_TOGGLE)).toBeTruthy();
} }
}); });