From 746a154f2bb70d2c2349bbb36babff28abf0eff0 Mon Sep 17 00:00:00 2001 From: kialam Date: Fri, 30 Nov 2018 11:23:15 -0500 Subject: [PATCH 1/4] Address missing job events. - Fix off by one error. - Add unit tests for Stream Service. --- .../client/features/output/stream.service.js | 9 +- awx/ui/test/unit/components/index.js | 2 +- awx/ui/test/unit/components/stream.unit.js | 96 +++++++++++++++++++ 3 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 awx/ui/test/unit/components/stream.unit.js diff --git a/awx/ui/client/features/output/stream.service.js b/awx/ui/client/features/output/stream.service.js index 9065d98517..11198e0752 100644 --- a/awx/ui/client/features/output/stream.service.js +++ b/awx/ui/client/features/output/stream.service.js @@ -50,6 +50,10 @@ function OutputStream ($q) { this.calcFactors = size => { const factors = [1]; + if (size !== parseInt(size, 10) || size <= 1) { + return factors; + } + for (let i = 2; i <= size / 2; i++) { if (size % i === 0) { factors.push(i); @@ -135,7 +139,7 @@ function OutputStream ($q) { this.isReadyToRender = () => { const { total } = this.counters; - const readyCount = this.counters.ready - this.counters.min; + const readyCount = this.getReadyCount(); if (readyCount <= 0) { return false; @@ -202,7 +206,7 @@ function OutputStream ($q) { return $q.resolve(); } - const readyCount = this.counters.ready - this.counters.min; + const readyCount = this.getReadyCount(); let events = []; if (readyCount > 0) { @@ -230,6 +234,7 @@ function OutputStream ($q) { }); this.getMaxCounter = () => this.counters.max; + this.getReadyCount = () => this.counters.ready - this.counters.min + 1; } OutputStream.$inject = ['$q']; diff --git a/awx/ui/test/unit/components/index.js b/awx/ui/test/unit/components/index.js index a95460d83a..16d45da9c7 100644 --- a/awx/ui/test/unit/components/index.js +++ b/awx/ui/test/unit/components/index.js @@ -8,4 +8,4 @@ import './side-nav.unit'; import './side-nav-item.unit'; import './jobs-list-split-jobs.unit'; import './job-details-split-jobs.unit'; - +import './stream.unit'; diff --git a/awx/ui/test/unit/components/stream.unit.js b/awx/ui/test/unit/components/stream.unit.js new file mode 100644 index 0000000000..78cec087aa --- /dev/null +++ b/awx/ui/test/unit/components/stream.unit.js @@ -0,0 +1,96 @@ +import StreamService from '~features/output/stream.service'; + +describe('Output | StreamService', () => { + angular.module('test', []).service('StreamService', StreamService); + let stream; + + beforeEach(() => { + angular.mock.module('test'); + }); + + beforeEach(angular.mock.inject(($injector) => { + stream = $injector.get('StreamService'); + + const onFrames = angular.noop; + const onFrameRate = angular.noop; + + stream.init({ onFrames, onFrameRate }); + })); + + describe('calcFactors', () => { + it('returns the expected values', () => { + const params = [ + [-1, [1]], + [0, [1]], + [1, [1]], + [1.0, [1]], + [1.1, [1]], + [2, [1, 2]], + ['1', [1]], + [{}, [1]], + [null, [1]], + [undefined, [1]], + [250, [1, 2, 5, 10, 25, 50, 125, 250]] + ]; + + params.forEach(([size, expected]) => expect(stream.calcFactors(size)).toEqual(expected)); + }); + }); + + describe('setMissingCounterThreshold', () => { + it('returns the correct counter threshold', () => { + const gt = 2; + stream.setMissingCounterThreshold(gt); + expect(stream.counters.min).toEqual(gt); + + const lt = -1; + stream.setMissingCounterThreshold(lt); + expect(stream.counters.min).toEqual(gt); + }); + }); + + describe('isReadyToRender', () => { + it('returns false', () => { + const res = stream.isReadyToRender(); + expect(res).toBe(false); + }); + + it('returns true', () => { + stream.state.ending = true; + let res = stream.isReadyToRender(); + expect(res).toBe(true); + + stream.counters.total = 1; + stream.framesPerRender = 1; + + res = stream.isReadyToRender(); + expect(res).toBe(true); + }); + }); + + describe('getMaxCounter', () => { + it('returns the same value as max counter', () => { + const res = stream.getMaxCounter(); + expect(res).toEqual(stream.counters.max); + }); + }); + + describe('getReadyCount', () => { + it('references min and max counters', () => { + expect(stream.getReadyCount()).toEqual(stream.counters.max - stream.counters.min + 1); + }); + it('returns expected values if min or max value is a non-integer', () => { + const params = [ + [null, 1, 0], + [undefined, 1, NaN], + ['1', 1, 1] + ]; + + params.forEach(([x, y, z]) => { + stream.counters.ready = x; + stream.counters.min = y; + expect(stream.getReadyCount()).toEqual(z); + }); + }); + }); +}); From 3e1e068013899c14bb0155930a8e663b5e6f3ddd Mon Sep 17 00:00:00 2001 From: kialam Date: Fri, 30 Nov 2018 12:03:26 -0500 Subject: [PATCH 2/4] Add boundary checks for `getReadyCount` method. --- awx/ui/test/unit/components/stream.unit.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/awx/ui/test/unit/components/stream.unit.js b/awx/ui/test/unit/components/stream.unit.js index 78cec087aa..de0779cccd 100644 --- a/awx/ui/test/unit/components/stream.unit.js +++ b/awx/ui/test/unit/components/stream.unit.js @@ -83,7 +83,10 @@ describe('Output | StreamService', () => { const params = [ [null, 1, 0], [undefined, 1, NaN], - ['1', 1, 1] + ['1', 1, 1], + [-1, -3, 3], + [0, 0, 1], + [6, 5, 2] ]; params.forEach(([x, y, z]) => { From 473ce95c863fb9cc261480b1f58d6ef71040fc73 Mon Sep 17 00:00:00 2001 From: kialam Date: Fri, 30 Nov 2018 12:16:28 -0500 Subject: [PATCH 3/4] Fix failing unit tests. --- awx/ui/test/unit/components/stream.unit.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/awx/ui/test/unit/components/stream.unit.js b/awx/ui/test/unit/components/stream.unit.js index de0779cccd..e51b2ef9da 100644 --- a/awx/ui/test/unit/components/stream.unit.js +++ b/awx/ui/test/unit/components/stream.unit.js @@ -56,6 +56,8 @@ describe('Output | StreamService', () => { }); it('returns true', () => { + stream.counters.min = 1; + stream.counters.ready = 2; stream.state.ending = true; let res = stream.isReadyToRender(); expect(res).toBe(true); From afa7c2d69fcd6fbf5a5a6b09c6c1d093ed76a9e3 Mon Sep 17 00:00:00 2001 From: kialam Date: Fri, 30 Nov 2018 13:58:13 -0500 Subject: [PATCH 4/4] Update unit tests. --- awx/ui/test/unit/components/stream.unit.js | 168 ++++++++++----------- 1 file changed, 82 insertions(+), 86 deletions(-) diff --git a/awx/ui/test/unit/components/stream.unit.js b/awx/ui/test/unit/components/stream.unit.js index e51b2ef9da..c4343d59b7 100644 --- a/awx/ui/test/unit/components/stream.unit.js +++ b/awx/ui/test/unit/components/stream.unit.js @@ -1,101 +1,97 @@ import StreamService from '~features/output/stream.service'; describe('Output | StreamService', () => { - angular.module('test', []).service('StreamService', StreamService); - let stream; + angular.module('test', []).service('StreamService', StreamService); + let stream; - beforeEach(() => { - angular.mock.module('test'); - }); - - beforeEach(angular.mock.inject(($injector) => { - stream = $injector.get('StreamService'); - - const onFrames = angular.noop; - const onFrameRate = angular.noop; - - stream.init({ onFrames, onFrameRate }); - })); - - describe('calcFactors', () => { - it('returns the expected values', () => { - const params = [ - [-1, [1]], - [0, [1]], - [1, [1]], - [1.0, [1]], - [1.1, [1]], - [2, [1, 2]], - ['1', [1]], - [{}, [1]], - [null, [1]], - [undefined, [1]], - [250, [1, 2, 5, 10, 25, 50, 125, 250]] - ]; - - params.forEach(([size, expected]) => expect(stream.calcFactors(size)).toEqual(expected)); - }); - }); - - describe('setMissingCounterThreshold', () => { - it('returns the correct counter threshold', () => { - const gt = 2; - stream.setMissingCounterThreshold(gt); - expect(stream.counters.min).toEqual(gt); - - const lt = -1; - stream.setMissingCounterThreshold(lt); - expect(stream.counters.min).toEqual(gt); - }); - }); - - describe('isReadyToRender', () => { - it('returns false', () => { - const res = stream.isReadyToRender(); - expect(res).toBe(false); + beforeEach(() => { + angular.mock.module('test'); }); - it('returns true', () => { - stream.counters.min = 1; - stream.counters.ready = 2; - stream.state.ending = true; - let res = stream.isReadyToRender(); - expect(res).toBe(true); + beforeEach(angular.mock.inject(($injector) => { + stream = $injector.get('StreamService'); - stream.counters.total = 1; - stream.framesPerRender = 1; + const onFrames = angular.noop; + const onFrameRate = angular.noop; - res = stream.isReadyToRender(); - expect(res).toBe(true); + stream.init({ onFrames, onFrameRate }); + })); + + describe('calcFactors', () => { + it('returns the expected values', () => { + const params = [ + [-1, [1]], + [0, [1]], + [1, [1]], + [1.0, [1]], + [1.1, [1]], + [2, [1, 2]], + ['1', [1]], + [{}, [1]], + [null, [1]], + [undefined, [1]], + [250, [1, 2, 5, 10, 25, 50, 125, 250]] + ]; + + params.forEach(([size, expected]) => + expect(stream.calcFactors(size)).toEqual(expected)); + }); }); - }); - describe('getMaxCounter', () => { - it('returns the same value as max counter', () => { - const res = stream.getMaxCounter(); - expect(res).toEqual(stream.counters.max); - }); - }); + describe('setMissingCounterThreshold', () => { + it('returns the correct counter threshold', () => { + const gt = 2; + stream.setMissingCounterThreshold(gt); + expect(stream.counters.min).toEqual(gt); - describe('getReadyCount', () => { - it('references min and max counters', () => { - expect(stream.getReadyCount()).toEqual(stream.counters.max - stream.counters.min + 1); + const lt = -1; + stream.setMissingCounterThreshold(lt); + expect(stream.counters.min).toEqual(gt); + }); }); - it('returns expected values if min or max value is a non-integer', () => { - const params = [ - [null, 1, 0], - [undefined, 1, NaN], - ['1', 1, 1], - [-1, -3, 3], - [0, 0, 1], - [6, 5, 2] - ]; - params.forEach(([x, y, z]) => { - stream.counters.ready = x; - stream.counters.min = y; - expect(stream.getReadyCount()).toEqual(z); - }); + describe('isReadyToRender', () => { + it("it's never ready to render unless the result of getReadyCount is greater than 0", () => { + const params = [ + [-1, false], + [0, false], + [1, true] + ]; + const spy = spyOn(stream, 'getReadyCount'); + + params.forEach(([readyCount, expected]) => { + spy.and.returnValue(readyCount); + expect(stream.isReadyToRender()).toEqual(expected); + }); + }); + }); + + describe('getMaxCounter', () => { + it('returns the same value as max counter', () => { + const res = stream.getMaxCounter(); + expect(res).toEqual(stream.counters.max); + }); + }); + + describe('getReadyCount', () => { + it('references min and max counters', () => { + expect(stream.getReadyCount()).toEqual(stream.counters.max - stream.counters.min + 1); + }); + it('returns expected values if min or max value is a non-integer', () => { + const params = [ + [null, 1, 0], + [undefined, 1, NaN], + ['1', 1, 1], + [-1, -3, 3], + [0, 0, 1], + [6, 5, 2] + ]; + + params.forEach(([max, min, expected]) => { + stream.counters.ready = max; + stream.counters.min = min; + expect(stream.getReadyCount()).toEqual(expected); + }); + }); }); - }); });