Skip to content

Commit 2c83115

Browse files
mttrbrtsmttrbrts
authored
chore(*): improve code coverage (#510)
* chore(*): improve code coverage Signed-off-by: Matt Roberts <[email protected]> * test(analysis): improve code coverage Signed-off-by: Matt Roberts <[email protected]> * test(core): add unit tests for modelfile Signed-off-by: Matt Roberts <[email protected]> Signed-off-by: Matt Roberts <[email protected]>
1 parent df10d15 commit 2c83115

File tree

17 files changed

+1481
-1457
lines changed

17 files changed

+1481
-1457
lines changed

‎packages/concerto-analysis/package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
"pretest": "npm-run-all licchk lint",
1717
"lint": "eslint .",
1818
"licchk": "license-check-and-add",
19-
"test": "jest"
19+
"test": "jest",
20+
"test:watch": "jest --watchAll"
2021
},
2122
"repository": {
2223
"type": "git",

‎packages/concerto-analysis/src/comparers/properties.ts‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,17 @@ const propertyTypeChanged: ComparerFactory = (context) => ({
140140
const bTypeFullNamespace = ModelUtil.getNamespace(bFQTN);
141141
if (!aTypeFullNamespace && !bTypeFullNamespace) {
142142
return;
143-
} else if (!aTypeFullNamespace || !bTypeFullNamespace) {
144-
context.report({
145-
key: 'property-type-changed',
146-
message: `The ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from "${aFQTN}" to "${bFQTN}" (type namespace differs)`,
147-
element: a
148-
});
149-
return;
150143
}
144+
// Removing until we support type aliasing. Until then it's not possible to have an empty namespace
145+
// (i.e. a primitive type) and to have the namespace change between versions
146+
// else if (!aTypeFullNamespace || !bTypeFullNamespace) {
147+
// context.report({
148+
// key: 'property-type-changed',
149+
// message: `The ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from "${aFQTN}" to "${bFQTN}" (type namespace differs)`,
150+
// element: a
151+
// });
152+
// return;
153+
// }
151154
const { name: aTypeNamespace, version: aTypeVersion } = ModelUtil.parseNamespace(aTypeFullNamespace);
152155
const { name: bTypeNamespace, version: bTypeVersion } = ModelUtil.parseNamespace(bTypeFullNamespace);
153156
if (aTypeNamespace !== bTypeNamespace) {
@@ -158,16 +161,17 @@ const propertyTypeChanged: ComparerFactory = (context) => ({
158161
});
159162
return;
160163
}
161-
if (!aTypeVersion && !bTypeVersion) {
162-
return;
163-
} else if (!aTypeVersion || !bTypeVersion) {
164-
context.report({
165-
key: 'property-type-changed',
166-
message: `The ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from "${aFQTN}" to "${bFQTN}" (type version incompatible)`,
167-
element: a
168-
});
169-
return;
170-
}
164+
// Removing until the Compare.compare function supports a non-strict modelManager
165+
// if (!aTypeVersion && !bTypeVersion) {
166+
// return;
167+
// } else if (!aTypeVersion || !bTypeVersion) {
168+
// context.report({
169+
// key: 'property-type-changed',
170+
// message: `The ${aType} "${a.getName()}" in the ${classDeclarationType} "${a.getParent().getName()}" changed type from "${aFQTN}" to "${bFQTN}" (type version incompatible)`,
171+
// element: a
172+
// });
173+
// return;
174+
// }
171175
const versionDiff = semver.diff(aTypeVersion, bTypeVersion);
172176
if (!versionDiff) {
173177
return;

‎packages/concerto-analysis/test/fixtures/field-namespace-changed-primitive-b.cto‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
namespace [email protected]
2+
3+
concept Thing {
4+
o Boolean bar
5+
}

‎packages/concerto-analysis/test/fixtures/field-namespace-changed-unversioned-a.cto‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
namespace [email protected]
2+
3+
import org.accordproject.concerto.test.other.Bar
4+
5+
concept Thing {
6+
o Bar bar
7+
}

‎packages/concerto-analysis/test/unit/compare-utils.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { ClassDeclaration, ModelFile, ModelManager, Property, Validator, Field } from '@accordproject/concerto-core';
2+
import { getClassDeclarationType, getPropertyType, getValidatorType } from '../../src/compare-utils';
3+
4+
// This test suite should disappear once we port concerto-core to TypeScript because the error branches will be enforced by the transpiler.
5+
6+
const modelManager = new ModelManager();
7+
const propertyAst = {
8+
name: 'myProp',
9+
type: 'Boolean'
10+
};
11+
const modelAst = {
12+
namespace: '[email protected]',
13+
properties: []
14+
};
15+
const modelFile = new ModelFile(modelManager, modelAst);
16+
const classDeclaration = new ClassDeclaration(modelFile, modelAst);
17+
const property = new Property(classDeclaration, propertyAst);
18+
const field = new Field(classDeclaration, propertyAst);
19+
const validator = new Validator(field, undefined);
20+
21+
test('should throw for unknown class declaration type', () => {
22+
expect(() => getClassDeclarationType(classDeclaration)).toThrow('unknown class declaration type "ClassDeclaration {[email protected] super=Concept enum=false abstract=false}"');
23+
});
24+
25+
test('should throw for unknown class property type', () => {
26+
expect(() => getPropertyType(property)).toThrow('unknown property type "[object Object]');
27+
});
28+
29+
test('should throw for unknown validator type', () => {
30+
expect(() => getValidatorType(validator)).toThrow('unknown validator type "[object Object]');
31+
});

‎packages/concerto-analysis/test/unit/compare.test.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ async function getModelFile(modelManager: ModelManager, fileName: string) {
1313

1414
async function getModelFiles(
1515
aFileName: string,
16-
bFileName: string
16+
bFileName: string,
1717
): Promise<[a: ModelFile, b: ModelFile]> {
1818
const modelManager = new ModelManager({ strict: true });
1919
const a = await getModelFile(modelManager, aFileName);
@@ -40,6 +40,22 @@ test('should detect no changes between two identical files', async () => {
4040
expect(results.result).toBe(CompareResult.NONE);
4141
});
4242

43+
test('should reject a non-strict modelManager, a', async () => {
44+
const modelManager = new ModelManager({ strict: false });
45+
const strictModelManager = new ModelManager({ strict: true });
46+
const a = await getModelFile(modelManager, 'identical.cto');
47+
const b = await getModelFile(strictModelManager, 'identical.cto');
48+
expect(() => new Compare().compare(a, b)).toThrow('model file "[email protected]" does not have strict versioned namespaces');
49+
});
50+
51+
test('should reject a non-strict modelManager, b', async () => {
52+
const modelManager = new ModelManager({ strict: true });
53+
const strictModelManager = new ModelManager({ strict: false });
54+
const a = await getModelFile(modelManager, 'identical.cto');
55+
const b = await getModelFile(strictModelManager, 'identical.cto');
56+
expect(() => new Compare().compare(a, b)).toThrow('model file "[email protected]" does not have strict versioned namespaces');
57+
});
58+
4359
test('should detect a change of namespace', async () => {
4460
const [a, b] = await getModelFiles('namespace-changed-a.cto', 'namespace-changed-b.cto');
4561
const results = new Compare().compare(a, b);

‎packages/concerto-core/lib/concerto.js‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,7 @@ class Concerto {
8484
throw new Error('Input object does not have a $class attribute.');
8585
}
8686

87-
const typeDeclaration = this.modelManager.getType(obj.$class);
88-
89-
if (!typeDeclaration) {
90-
throw new Error(`Type ${obj.$class} is not declared in the model manager`);
91-
}
92-
93-
return typeDeclaration;
87+
return this.modelManager.getType(obj.$class);
9488
}
9589

9690
/**

‎packages/concerto-core/lib/serializer/jsonpopulator.js‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -338,11 +338,6 @@ class JSONPopulator {
338338

339339
if(relationshipDeclaration.isArray()) {
340340
result = [];
341-
if (this.ergo) {
342-
if (Object.prototype.hasOwnProperty.call(jsonObj,'$coll')) {
343-
jsonObj = jsonObj.$coll.slice(0,jsonObj.$length);
344-
}
345-
}
346341
for(let n=0; n < jsonObj.length; n++) {
347342
let jsonItem = jsonObj[n];
348343
if (typeof jsonItem === 'string') {
@@ -358,9 +353,9 @@ class JSONPopulator {
358353
}
359354

360355
if (this.ergo) {
361-
const theClass = jsonObj.$class.$coll[0];
362-
jsonObj = jsonObj.$data;
363-
jsonObj.$class = theClass;
356+
const theClass = jsonItem.$class.$coll[0];
357+
jsonItem = jsonItem.$data;
358+
jsonItem.$class = theClass;
364359
}
365360
const classDeclaration = parameters.modelManager.getType(jsonItem.$class);
366361

@@ -386,7 +381,6 @@ class JSONPopulator {
386381
if(!jsonObj.$class) {
387382
throw new Error('Invalid JSON data. Does not contain a $class type identifier: ' + jsonObj + ' for relationship ' + relationshipDeclaration );
388383
}
389-
390384
if (this.ergo) {
391385
const theClass = jsonObj.$class.$coll[0];
392386
jsonObj = jsonObj.$data;

‎packages/concerto-core/lib/serializer/objectvalidator.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,8 @@ class ObjectValidator {
410410
let typeOfValue = typeof value;
411411

412412
if( concerto.isObject(value) && concerto.isIdentifiable(value)) {
413-
typeOfValue = value.getFullyQualifiedType();
414-
value = value.getFullyQualifiedIdentifier();
413+
typeOfValue = concerto.getType(value);
414+
value = concerto.getIdentifier(value);
415415
}
416416
else {
417417
if(value) {

‎packages/concerto-core/test/concerto.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,18 @@ describe('concerto', () => {
8888
}).should.throw(/Object does not have an identifier/);
8989
});
9090

91+
it('should throw for undeclared type', () => {
92+
const obj = {
93+
$class : 'org.accordproject.test.BadProduct',
94+
sku: '001',
95+
description: 'Widgets'
96+
};
97+
98+
(() => {
99+
concerto.getIdentifier(obj);
100+
}).should.throw(/Type "BadProduct" is not defined in namespace "org.accordproject.test"/);
101+
});
102+
91103
});
92104

93105
describe('#setIdentifier', () => {
@@ -146,6 +158,24 @@ describe('concerto', () => {
146158
result.typeDeclaration.getName().should.equal('Person');
147159
result.id.should.equal('123456789');
148160
});
161+
162+
it('should throw for invalid URI', () => {
163+
(() =>
164+
concerto.fromURI(1)
165+
).should.throw('Invalid URI: 1');
166+
});
167+
168+
it('should throw for invalid URI scheme', () => {
169+
(() =>
170+
concerto.fromURI('bad://uri')
171+
).should.throw('Invalid URI scheme: bad://uri');
172+
});
173+
174+
it('should throw for invalid URI scheme', () => {
175+
(() =>
176+
concerto.fromURI('resource://uri?query')
177+
).should.throw('Invalid resource URI format: resource://uri?query');
178+
});
149179
});
150180

151181
describe('#getTypeDeclaration', () => {

0 commit comments

Comments
 (0)