From 3f49ddc05386d5dce6391166d30a3cd91c5cb227 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Fri, 12 Jul 2024 13:33:17 +0400 Subject: [PATCH 1/3] test: improve test coverage for DataProviderController --- .../data-provider-controller-cache.test.js | 67 +++++++++++++++++-- ...ta-provider-controller-placeholder.test.js | 0 .../test/data-provider-controller.test.js | 39 +++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 packages/component-base/test/data-provider-controller-placeholder.test.js diff --git a/packages/component-base/test/data-provider-controller-cache.test.js b/packages/component-base/test/data-provider-controller-cache.test.js index daec383f49..4d17ae2720 100644 --- a/packages/component-base/test/data-provider-controller-cache.test.js +++ b/packages/component-base/test/data-provider-controller-cache.test.js @@ -1,6 +1,8 @@ import { expect } from '@esm-bundle/chai'; import { Cache } from '../src/data-provider-controller/cache.js'; +const PLACEHOLDER = Symbol.for('PLACEHOLDER'); + describe('DataProviderController - Cache', () => { let cache; let expandedItems = []; @@ -51,23 +53,42 @@ describe('DataProviderController - Cache', () => { }); }); + describe('with placeholder', () => { + beforeEach(() => { + cache = new Cache({ isExpanded, placeholder: PLACEHOLDER }, 2, 20); + }); + + it('should have items filled with placeholder', () => { + expect(cache.items).to.have.lengthOf(20); + expect(cache.items.every((item) => item === PLACEHOLDER)).to.be.true; + }); + }); + describe('setPage', () => { beforeEach(() => { - cache = new Cache({ isExpanded }, 50, 500); + cache = new Cache({ isExpanded }, 2, 20); }); - it('should insert the items at the correct position for page 0', () => { + it('should insert items at the correct position for page 0', () => { cache.setPage(0, ['Item 0', 'Item 1']); expect(cache.items).to.have.lengthOf(2); expect(cache.items[0]).to.equal('Item 0'); expect(cache.items[1]).to.equal('Item 1'); }); - it('should insert the items at the correct position for page 1', () => { - cache.setPage(1, ['Item 0', 'Item 1']); - expect(cache.items).to.have.lengthOf(52); - expect(cache.items[50]).to.equal('Item 0'); - expect(cache.items[51]).to.equal('Item 1'); + it('should insert items at the correct position for page 1', () => { + cache.setPage(1, ['Item 2', 'Item 3']); + expect(cache.items).to.have.lengthOf(4); + expect(cache.items[2]).to.equal('Item 2'); + expect(cache.items[3]).to.equal('Item 3'); + }); + + it('should discard items that exceed the size bounds', () => { + cache.setPage(9, ['Item 18', 'Item 19', 'Item 20']); + expect(cache.items).to.have.lengthOf(20); + expect(cache.items[18]).to.equal('Item 18'); + expect(cache.items[19]).to.equal('Item 19'); + expect(cache.items[20]).to.be.undefined; }); }); @@ -197,6 +218,38 @@ describe('DataProviderController - Cache', () => { }); }); + describe('size', () => { + beforeEach(() => { + cache = new Cache({ isExpanded }, 2, 20); + }); + + it('should remove exceeding pending requests when decreasing size', () => { + cache.pendingRequests[8] = (_items, _size) => {}; + cache.pendingRequests[9] = (_items, _size) => {}; + cache.size = 18; + expect(cache.pendingRequests[8]).to.be.not.undefined; + expect(cache.pendingRequests[9]).to.be.undefined; + }); + }); + + describe('size with placeholder', () => { + beforeEach(() => { + cache = new Cache({ isExpanded, placeholder: PLACEHOLDER }, 2, 20); + }); + + it('should add more placeholders when increasing size', () => { + cache.size = 22; + expect(cache.items).to.have.lengthOf(22); + expect(cache.items.every((item) => item === PLACEHOLDER)).to.be.true; + }); + + it('should remove exceeding placeholders when decreasing size', () => { + cache.size = 18; + expect(cache.items).to.have.lengthOf(18); + expect(cache.items.every((item) => item === PLACEHOLDER)).to.be.true; + }); + }); + describe('flatSize', () => { beforeEach(() => { cache = new Cache({ isExpanded }, 50, 500); diff --git a/packages/component-base/test/data-provider-controller-placeholder.test.js b/packages/component-base/test/data-provider-controller-placeholder.test.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/component-base/test/data-provider-controller.test.js b/packages/component-base/test/data-provider-controller.test.js index 8e486f3fec..03ccd17a4f 100644 --- a/packages/component-base/test/data-provider-controller.test.js +++ b/packages/component-base/test/data-provider-controller.test.js @@ -5,6 +5,8 @@ import { Cache } from '../src/data-provider-controller/cache.js'; import { DataProviderController } from '../src/data-provider-controller/data-provider-controller.js'; import { createDataProvider } from './data-provider-controller-helpers.js'; +const PLACEHOLDER = Symbol.for('PLACEHOLDER'); + describe('DataProviderController', () => { let host, controller; @@ -47,10 +49,22 @@ describe('DataProviderController', () => { expect(controller.flatSize).to.equal(0); }); + it('should not have placeholder', () => { + expect(controller.placeholder).to.be.undefined; + }); + + it('should not have isPlaceholder', () => { + expect(controller.isPlaceholder).to.be.undefined; + }); + it('should have rootCache', () => { expect(controller.rootCache).to.be.instanceOf(Cache); }); + it('should have empty rootCache items', () => { + expect(controller.rootCache.items).to.have.lengthOf(0); + }); + it('should not have rootCache size', () => { expect(controller.rootCache.size).to.be.undefined; }); @@ -85,6 +99,31 @@ describe('DataProviderController', () => { it('should have rootCache size', () => { expect(controller.rootCache.size).to.equal(500); }); + + it('should have empty rootCache items', () => { + expect(controller.rootCache.items).to.have.lengthOf(0); + }); + }); + + describe('with placeholder', () => { + beforeEach(() => { + controller = new DataProviderController(host, { + size: 100, + pageSize: 50, + isExpanded, + placeholder: PLACEHOLDER, + dataProvider: (_params, callback) => callback([], 0), + }); + }); + + it('should have placeholder', () => { + expect(controller.placeholder).to.equal(PLACEHOLDER); + }); + + it('should have rootCache items filled with placeholder', () => { + expect(controller.rootCache.items).to.have.lengthOf(100); + expect(controller.rootCache.items.every((item) => item === PLACEHOLDER)).to.be.true; + }); }); describe('with dataProviderParams', () => { From 2d0c88121d340e012936ff13777836b5c711149f Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Fri, 12 Jul 2024 14:56:51 +0400 Subject: [PATCH 2/3] add more tests --- ...ta-provider-controller-placeholder.test.js | 83 +++++++++++++++++++ .../test/data-provider-controller.test.js | 21 ----- 2 files changed, 83 insertions(+), 21 deletions(-) diff --git a/packages/component-base/test/data-provider-controller-placeholder.test.js b/packages/component-base/test/data-provider-controller-placeholder.test.js index e69de29bb2..c85f366c68 100644 --- a/packages/component-base/test/data-provider-controller-placeholder.test.js +++ b/packages/component-base/test/data-provider-controller-placeholder.test.js @@ -0,0 +1,83 @@ +import { expect } from '@esm-bundle/chai'; +import sinon from 'sinon'; +import '@vaadin/testing-helpers'; +import { DataProviderController } from '../src/data-provider-controller/data-provider-controller.js'; +import { createDataProvider } from './data-provider-controller-helpers.js'; + +class Placeholder {} + +describe('DataProviderController - placeholder', () => { + let host, controller, placeholder, dataProviderSpy; + + describe('default', () => { + beforeEach(() => { + host = document.createElement('div'); + + placeholder = new Placeholder(); + + controller = new DataProviderController(host, { + size: 10, + pageSize: 2, + placeholder, + dataProvider: createDataProvider({ size: 10, async: true }), + }); + + dataProviderSpy = sinon.spy(controller, 'dataProvider'); + }); + + it('should have placeholder', () => { + expect(controller.placeholder).to.equal(placeholder); + }); + + it('should have rootCache items filled with placeholders', () => { + expect(controller.rootCache.items).to.have.lengthOf(10); + expect(controller.rootCache.items.every((item) => item === placeholder)).to.be.true; + }); + + it('should request data when encountering the controller placeholder instance', () => { + controller.ensureFlatIndexLoaded(0); + expect(dataProviderSpy).to.have.callCount(1); + + controller.ensureFlatIndexLoaded(1); + expect(dataProviderSpy).to.have.callCount(1); + + controller.ensureFlatIndexLoaded(2); + expect(dataProviderSpy).to.have.callCount(2); + + controller.ensureFlatIndexLoaded(3); + expect(dataProviderSpy).to.have.callCount(2); + }); + + it('should not request data when encountering a different placeholder instance', () => { + controller.rootCache.items[0] = new Placeholder(); + controller.ensureFlatIndexLoaded(0); + expect(dataProviderSpy).to.have.callCount(0); + }); + }); + + describe('custom equality function', () => { + beforeEach(() => { + host = document.createElement('div'); + + controller = new DataProviderController(host, { + size: 10, + pageSize: 2, + placeholder: new Placeholder(), + isPlaceholder: (item) => item instanceof Placeholder, + dataProvider: createDataProvider({ size: 10, async: true }), + }); + + dataProviderSpy = sinon.spy(controller, 'dataProvider'); + }); + + it('should request data when encountering any placeholder instance', () => { + controller.rootCache.items[0] = new Placeholder(); + + controller.ensureFlatIndexLoaded(0); + expect(dataProviderSpy).to.have.callCount(1); + + controller.ensureFlatIndexLoaded(2); + expect(dataProviderSpy).to.have.callCount(2); + }); + }); +}); diff --git a/packages/component-base/test/data-provider-controller.test.js b/packages/component-base/test/data-provider-controller.test.js index 03ccd17a4f..7c7f7d5a50 100644 --- a/packages/component-base/test/data-provider-controller.test.js +++ b/packages/component-base/test/data-provider-controller.test.js @@ -105,27 +105,6 @@ describe('DataProviderController', () => { }); }); - describe('with placeholder', () => { - beforeEach(() => { - controller = new DataProviderController(host, { - size: 100, - pageSize: 50, - isExpanded, - placeholder: PLACEHOLDER, - dataProvider: (_params, callback) => callback([], 0), - }); - }); - - it('should have placeholder', () => { - expect(controller.placeholder).to.equal(PLACEHOLDER); - }); - - it('should have rootCache items filled with placeholder', () => { - expect(controller.rootCache.items).to.have.lengthOf(100); - expect(controller.rootCache.items.every((item) => item === PLACEHOLDER)).to.be.true; - }); - }); - describe('with dataProviderParams', () => { let dataProviderParams; From 03de94e79779a16e246c9b69fafe6dcdfaba1483 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Fri, 12 Jul 2024 15:19:06 +0400 Subject: [PATCH 3/3] wip --- .../test/data-provider-controller-cache.test.js | 4 ++-- .../test/data-provider-controller-placeholder.test.js | 6 ++++-- .../component-base/test/data-provider-controller.test.js | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/component-base/test/data-provider-controller-cache.test.js b/packages/component-base/test/data-provider-controller-cache.test.js index 4d17ae2720..e1460d45cb 100644 --- a/packages/component-base/test/data-provider-controller-cache.test.js +++ b/packages/component-base/test/data-provider-controller-cache.test.js @@ -1,7 +1,7 @@ import { expect } from '@esm-bundle/chai'; import { Cache } from '../src/data-provider-controller/cache.js'; -const PLACEHOLDER = Symbol.for('PLACEHOLDER'); +const PLACEHOLDER = Symbol('PLACEHOLDER'); describe('DataProviderController - Cache', () => { let cache; @@ -58,7 +58,7 @@ describe('DataProviderController - Cache', () => { cache = new Cache({ isExpanded, placeholder: PLACEHOLDER }, 2, 20); }); - it('should have items filled with placeholder', () => { + it('should have items filled with placeholders', () => { expect(cache.items).to.have.lengthOf(20); expect(cache.items.every((item) => item === PLACEHOLDER)).to.be.true; }); diff --git a/packages/component-base/test/data-provider-controller-placeholder.test.js b/packages/component-base/test/data-provider-controller-placeholder.test.js index c85f366c68..d9bd3e3930 100644 --- a/packages/component-base/test/data-provider-controller-placeholder.test.js +++ b/packages/component-base/test/data-provider-controller-placeholder.test.js @@ -6,6 +6,8 @@ import { createDataProvider } from './data-provider-controller-helpers.js'; class Placeholder {} +class CustomPlaceholder extends Placeholder {} + describe('DataProviderController - placeholder', () => { let host, controller, placeholder, dataProviderSpy; @@ -49,7 +51,7 @@ describe('DataProviderController - placeholder', () => { }); it('should not request data when encountering a different placeholder instance', () => { - controller.rootCache.items[0] = new Placeholder(); + controller.rootCache.items[0] = new CustomPlaceholder(); controller.ensureFlatIndexLoaded(0); expect(dataProviderSpy).to.have.callCount(0); }); @@ -71,7 +73,7 @@ describe('DataProviderController - placeholder', () => { }); it('should request data when encountering any placeholder instance', () => { - controller.rootCache.items[0] = new Placeholder(); + controller.rootCache.items[0] = new CustomPlaceholder(); controller.ensureFlatIndexLoaded(0); expect(dataProviderSpy).to.have.callCount(1); diff --git a/packages/component-base/test/data-provider-controller.test.js b/packages/component-base/test/data-provider-controller.test.js index 7c7f7d5a50..61db6bb566 100644 --- a/packages/component-base/test/data-provider-controller.test.js +++ b/packages/component-base/test/data-provider-controller.test.js @@ -5,7 +5,7 @@ import { Cache } from '../src/data-provider-controller/cache.js'; import { DataProviderController } from '../src/data-provider-controller/data-provider-controller.js'; import { createDataProvider } from './data-provider-controller-helpers.js'; -const PLACEHOLDER = Symbol.for('PLACEHOLDER'); +const PLACEHOLDER = Symbol('PLACEHOLDER'); describe('DataProviderController', () => { let host, controller;