From: Jukka Kurkela Date: Mon, 17 Aug 2020 14:03:15 +0000 (+0300) Subject: Improve test coverage and fix minor issues found (#7713) X-Git-Tag: v3.0.0-beta.2~11 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f5b4a0fa3c933ac443b66cfa3e0198f4f342a410;p=thirdparty%2FChart.js.git Improve test coverage and fix minor issues found (#7713) * Registry * Element * Animation * Animations * Animator --- diff --git a/src/core/core.animations.js b/src/core/core.animations.js index 343a67ecc..c1b17687f 100644 --- a/src/core/core.animations.js +++ b/src/core/core.animations.js @@ -105,7 +105,9 @@ export default class Animations { animatedProps.set(prop, Object.assign({}, animDefaults, cfg)); } else if (prop === key) { // Single property targetting config wins over multi-targetting. - animatedProps.set(prop, Object.assign({}, animatedProps.get(prop), cfg)); + // eslint-disable-next-line no-unused-vars + const {properties, ...inherited} = animatedProps.get(prop); + animatedProps.set(prop, Object.assign({}, inherited, cfg)); } }); }); diff --git a/src/core/core.registry.js b/src/core/core.registry.js index e71e50ba4..d6f23f5d9 100644 --- a/src/core/core.registry.js +++ b/src/core/core.registry.js @@ -90,6 +90,34 @@ export class Registry { return this._get(id, this.scales, 'scale'); } + /** + * @param {...typeof DatasetController} args + */ + removeControllers(...args) { + this._each('unregister', args, this.controllers); + } + + /** + * @param {...typeof Element} args + */ + removeElements(...args) { + this._each('unregister', args, this.elements); + } + + /** + * @param {...any} args + */ + removePlugins(...args) { + this._each('unregister', args, this.plugins); + } + + /** + * @param {...typeof Scale} args + */ + removeScales(...args) { + this._each('unregister', args, this.scales); + } + /** * @private */ @@ -97,7 +125,7 @@ export class Registry { const me = this; [...args].forEach(arg => { const reg = typedRegistry || me._getRegistryForType(arg); - if (reg.isForType(arg) || (reg === me.plugins && arg.id)) { + if (typedRegistry || reg.isForType(arg) || (reg === me.plugins && arg.id)) { me._exec(method, reg, arg); } else { // Handle loopable args diff --git a/src/core/core.typedRegistry.js b/src/core/core.typedRegistry.js index c7b72e53c..7e70cb112 100644 --- a/src/core/core.typedRegistry.js +++ b/src/core/core.typedRegistry.js @@ -42,6 +42,10 @@ export default class TypedRegistry { return scope; } + if (Object.keys(defaults.get(scope)).length) { + throw new Error('Can not register "' + id + '", because "defaults.' + scope + '" would collide with existing defaults'); + } + items[id] = item; registerDefaults(item, scope, parentScope); diff --git a/test/specs/core.animation.tests.js b/test/specs/core.animation.tests.js new file mode 100644 index 000000000..ed566f4cb --- /dev/null +++ b/test/specs/core.animation.tests.js @@ -0,0 +1,86 @@ +describe('Chart.Animation', function() { + it('should animate boolean', function() { + const target = {prop: false}; + const anim = new Chart.Animation({duration: 1000}, target, 'prop', true); + expect(anim.active()).toBeTrue(); + + anim.tick(anim._start + 500); + expect(anim.active()).toBeTrue(); + expect(target.prop).toBeFalse(); + + anim.tick(anim._start + 501); + expect(anim.active()).toBeTrue(); + expect(target.prop).toBeTrue(); + + anim.tick(anim._start - 100); + expect(anim.active()).toBeTrue(); + expect(target.prop).toBeFalse(); + + anim.tick(anim._start + 1000); + expect(anim.active()).toBeFalse(); + expect(target.prop).toBeTrue(); + }); + + describe('color', function() { + it('should fall back to transparent', function() { + const target = {}; + const anim = new Chart.Animation({duration: 1000, type: 'color'}, target, 'color', 'red'); + anim._from = undefined; + anim.tick(anim._start + 500); + expect(target.color).toEqual('#FF000080'); + + anim._from = 'blue'; + anim._to = undefined; + anim.tick(anim._start + 500); + expect(target.color).toEqual('#0000FF80'); + }); + + it('should not try to mix invalid color', function() { + const target = {color: 'blue'}; + const anim = new Chart.Animation({duration: 1000, type: 'color'}, target, 'color', 'invalid'); + anim.tick(anim._start + 500); + expect(target.color).toEqual('invalid'); + }); + }); + + it('should loop', function() { + const target = {value: 0}; + const anim = new Chart.Animation({duration: 100, loop: true}, target, 'value', 10); + anim.tick(anim._start + 50); + expect(target.value).toEqual(5); + anim.tick(anim._start + 100); + expect(target.value).toEqual(10); + anim.tick(anim._start + 150); + expect(target.value).toEqual(5); + anim.tick(anim._start + 400); + expect(target.value).toEqual(0); + }); + + it('should update', function() { + const target = {testColor: 'transparent'}; + const anim = new Chart.Animation({duration: 100, type: 'color'}, target, 'testColor', 'red'); + + anim.tick(anim._start + 50); + expect(target.testColor).toEqual('#FF000080'); + + anim.update({duration: 500}, 'blue', Date.now()); + anim.tick(anim._start + 250); + expect(target.testColor).toEqual('#4000BFBF'); + + anim.tick(anim._start + 500); + expect(target.testColor).toEqual('blue'); + }); + + it('should not update when finished', function() { + const target = {testColor: 'transparent'}; + const anim = new Chart.Animation({duration: 100, type: 'color'}, target, 'testColor', 'red'); + + anim.tick(anim._start + 100); + expect(target.testColor).toEqual('red'); + expect(anim.active()).toBeFalse(); + + anim.update({duration: 500}, 'blue', Date.now()); + expect(anim._duration).toEqual(100); + expect(anim._to).toEqual('red'); + }); +}); diff --git a/test/specs/core.animations.tests.js b/test/specs/core.animations.tests.js new file mode 100644 index 000000000..cc668291c --- /dev/null +++ b/test/specs/core.animations.tests.js @@ -0,0 +1,46 @@ +describe('Chart.animations', function() { + it('should override property collection with property', function() { + const chart = {}; + const anims = new Chart.Animations(chart, { + collection1: { + properties: ['property1', 'property2'], + duration: 1000 + }, + property2: { + duration: 2000 + } + }); + expect(anims._properties.get('property1')).toEqual(jasmine.objectContaining({duration: 1000})); + expect(anims._properties.get('property2')).toEqual({duration: 2000}); + }); + + it('should ignore duplicate definitions from collections', function() { + const chart = {}; + const anims = new Chart.Animations(chart, { + collection1: { + properties: ['property1'], + duration: 1000 + }, + collection2: { + properties: ['property1', 'property2'], + duration: 2000 + } + }); + expect(anims._properties.get('property1')).toEqual(jasmine.objectContaining({duration: 1000})); + expect(anims._properties.get('property2')).toEqual(jasmine.objectContaining({duration: 2000})); + }); + + it('should not animate undefined options key', function() { + const chart = {}; + const anims = new Chart.Animations(chart, {value: {duration: 100}, option: {duration: 200}}); + const target = { + value: 1, + options: { + option: 2 + } + }; + expect(anims.update(target, { + options: undefined + })).toBeUndefined(); + }); +}); diff --git a/test/specs/core.animator.tests.js b/test/specs/core.animator.tests.js index ea68235a7..cb05c139b 100644 --- a/test/specs/core.animator.tests.js +++ b/test/specs/core.animator.tests.js @@ -37,4 +37,12 @@ describe('Chart.animator', function() { }, }); }); + + it('should not fail when adding no items', function() { + const chart = {}; + Chart.animator.add(chart, undefined); + Chart.animator.add(chart, []); + Chart.animator.start(chart); + expect(Chart.animator.running(chart)).toBeFalse(); + }); }); diff --git a/test/specs/core.element.tests.js b/test/specs/core.element.tests.js new file mode 100644 index 000000000..f312f3df6 --- /dev/null +++ b/test/specs/core.element.tests.js @@ -0,0 +1,16 @@ +describe('Chart.element', function() { + describe('getProps', function() { + it('should return requested properties', function() { + const elem = new Chart.Element(); + elem.x = 10; + elem.y = 1.5; + + expect(elem.getProps(['x', 'y'])).toEqual(jasmine.objectContaining({x: 10, y: 1.5})); + expect(elem.getProps(['x', 'y'], true)).toEqual(jasmine.objectContaining({x: 10, y: 1.5})); + + elem.$animations = {x: {active: true, _to: 20}}; + expect(elem.getProps(['x', 'y'])).toEqual(jasmine.objectContaining({x: 10, y: 1.5})); + expect(elem.getProps(['x', 'y'], true)).toEqual(jasmine.objectContaining({x: 20, y: 1.5})); + }); + }); +}); diff --git a/test/specs/core.registry.tests.js b/test/specs/core.registry.tests.js index 6ed0b5dad..fbc6e50b8 100644 --- a/test/specs/core.registry.tests.js +++ b/test/specs/core.registry.tests.js @@ -64,7 +64,7 @@ describe('Chart.registry', function() { expect(Chart.defaults.elements.myElement).not.toBeDefined(); }); - it('should handle a classig plugin', function() { + it('should handle a classic plugin', function() { const CustomPlugin = { id: 'customPlugin', defaults: { @@ -172,4 +172,169 @@ describe('Chart.registry', function() { Chart.register(FaultyPlugin); }).toThrow(new Error('class does not have id: class FaultyPlugin {}')); }); + + it('should not fail when unregistering an object that is not registered', function() { + expect(function() { + Chart.unregister({id: 'foo'}); + }).not.toThrow(); + }); + + describe('Should allow registering explicitly', function() { + class customExtension {} + customExtension.id = 'custom'; + customExtension.defaults = { + prop: true + }; + + it('as controller', function() { + Chart.registry.addControllers(customExtension); + + expect(Chart.registry.getController('custom')).toEqual(customExtension); + expect(Chart.defaults.custom).toEqual(customExtension.defaults); + + Chart.registry.removeControllers(customExtension); + + expect(function() { + Chart.registry.getController('custom'); + }).toThrow(new Error('"custom" is not a registered controller.')); + expect(Chart.defaults.custom).not.toBeDefined(); + }); + + it('as scale', function() { + Chart.registry.addScales(customExtension); + + expect(Chart.registry.getScale('custom')).toEqual(customExtension); + expect(Chart.defaults.scales.custom).toEqual(customExtension.defaults); + + Chart.registry.removeScales(customExtension); + + expect(function() { + Chart.registry.getScale('custom'); + }).toThrow(new Error('"custom" is not a registered scale.')); + expect(Chart.defaults.scales.custom).not.toBeDefined(); + }); + + it('as element', function() { + Chart.registry.addElements(customExtension); + + expect(Chart.registry.getElement('custom')).toEqual(customExtension); + expect(Chart.defaults.elements.custom).toEqual(customExtension.defaults); + + Chart.registry.removeElements(customExtension); + + expect(function() { + Chart.registry.getElement('custom'); + }).toThrow(new Error('"custom" is not a registered element.')); + expect(Chart.defaults.elements.custom).not.toBeDefined(); + }); + + it('as plugin', function() { + Chart.registry.addPlugins(customExtension); + + expect(Chart.registry.getPlugin('custom')).toEqual(customExtension); + expect(Chart.defaults.plugins.custom).toEqual(customExtension.defaults); + + Chart.registry.removePlugins(customExtension); + + expect(function() { + Chart.registry.getPlugin('custom'); + }).toThrow(new Error('"custom" is not a registered plugin.')); + expect(Chart.defaults.plugins.custom).not.toBeDefined(); + }); + }); + + it('should fire before/after callbacks', function() { + let beforeRegisterCount = 0; + let afterRegisterCount = 0; + let beforeUnregisterCount = 0; + let afterUnregisterCount = 0; + class custom {} + custom.id = 'custom'; + custom.beforeRegister = () => beforeRegisterCount++; + custom.afterRegister = () => afterRegisterCount++; + custom.beforeUnregister = () => beforeUnregisterCount++; + custom.afterUnregister = () => afterUnregisterCount++; + + Chart.registry.addControllers(custom); + expect(beforeRegisterCount).withContext('beforeRegister').toBe(1); + expect(afterRegisterCount).withContext('afterRegister').toBe(1); + Chart.registry.removeControllers(custom); + expect(beforeUnregisterCount).withContext('beforeUnregister').toBe(1); + expect(afterUnregisterCount).withContext('afterUnregister').toBe(1); + + Chart.registry.addScales(custom); + expect(beforeRegisterCount).withContext('beforeRegister').toBe(2); + expect(afterRegisterCount).withContext('afterRegister').toBe(2); + Chart.registry.removeScales(custom); + expect(beforeUnregisterCount).withContext('beforeUnregister').toBe(2); + expect(afterUnregisterCount).withContext('afterUnregister').toBe(2); + + Chart.registry.addElements(custom); + expect(beforeRegisterCount).withContext('beforeRegister').toBe(3); + expect(afterRegisterCount).withContext('afterRegister').toBe(3); + Chart.registry.removeElements(custom); + expect(beforeUnregisterCount).withContext('beforeUnregister').toBe(3); + expect(afterUnregisterCount).withContext('afterUnregister').toBe(3); + + Chart.register(custom); + expect(beforeRegisterCount).withContext('beforeRegister').toBe(4); + expect(afterRegisterCount).withContext('afterRegister').toBe(4); + Chart.unregister(custom); + expect(beforeUnregisterCount).withContext('beforeUnregister').toBe(4); + expect(afterUnregisterCount).withContext('afterUnregister').toBe(4); + }); + + it('should not register anything that would collide with existing defaults', function() { + class testController extends Chart.DatasetController {} + testController.id = 'events'; + expect(function() { + Chart.register(testController); + }).toThrow(new Error('Can not register "events", because "defaults.events" would collide with existing defaults')); + }); + + describe('should handle multiple items', function() { + class test1 extends Chart.DatasetController {} + test1.id = 'test1'; + class test2 extends Chart.Scale {} + test2.id = 'test2'; + + it('separately', function() { + Chart.register(test1, test2); + expect(Chart.registry.getController('test1')).toEqual(test1); + expect(Chart.registry.getScale('test2')).toEqual(test2); + Chart.unregister(test1, test2); + expect(function() { + Chart.registry.getController('test1'); + }).toThrow(); + expect(function() { + Chart.registry.getScale('test2'); + }).toThrow(); + }); + + it('as array', function() { + Chart.register([test1, test2]); + expect(Chart.registry.getController('test1')).toEqual(test1); + expect(Chart.registry.getScale('test2')).toEqual(test2); + Chart.unregister([test1, test2]); + expect(function() { + Chart.registry.getController('test1'); + }).toThrow(); + expect(function() { + Chart.registry.getScale('test2'); + }).toThrow(); + }); + + it('as object', function() { + Chart.register({test1, test2}); + expect(Chart.registry.getController('test1')).toEqual(test1); + expect(Chart.registry.getScale('test2')).toEqual(test2); + Chart.unregister({test1, test2}); + expect(function() { + Chart.registry.getController('test1'); + }).toThrow(); + expect(function() { + Chart.registry.getScale('test2'); + }).toThrow(); + }); + }); });