From f792d30280bfcbc34779bb1977760008e439b33e Mon Sep 17 00:00:00 2001 From: Mateusz Krzeszowiak Date: Thu, 28 May 2020 08:46:04 +0200 Subject: [PATCH 1/7] Load color picker dependencies only when it is actually used --- .../js/lib/knockout/bindings/color-picker.js | 28 +++--- .../base/js/lib/ko/bind/color-picker.test.js | 97 ++++++++++--------- 2 files changed, 64 insertions(+), 61 deletions(-) diff --git a/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js b/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js index 1e8e89894d22f..b222eec45691c 100644 --- a/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js +++ b/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js @@ -5,10 +5,8 @@ define([ 'ko', 'jquery', - '../template/renderer', - 'spectrum', - 'tinycolor' -], function (ko, $, renderer, spectrum, tinycolor) { + '../template/renderer' +], function (ko, $, renderer) { 'use strict'; /** @@ -54,9 +52,11 @@ define([ return true; }; - $(element).spectrum(config); + require(['tinycolor', 'spectrum'], function() { + $(element).spectrum(config); - changeColorPickerStateBasedOnViewModel(element, viewModel); + changeColorPickerStateBasedOnViewModel(element, viewModel); + }); }, /** @@ -76,15 +76,17 @@ define([ config.value(''); } - if (tinycolor(config.value()).isValid() || config.value() === '') { - $(element).spectrum('set', config.value()); + require(['tinycolor', 'spectrum'], function(tinycolor) { + if (tinycolor(config.value()).isValid() || config.value() === '') { + $(element).spectrum('set', config.value()); - if (config.value() !== '') { - config.value($(element).spectrum('get').toString()); + if (config.value() !== '') { + config.value($(element).spectrum('get').toString()); + } } - } - - changeColorPickerStateBasedOnViewModel(element, viewModel); + + changeColorPickerStateBasedOnViewModel(element, viewModel); + }); } }; diff --git a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js index afbd7e8e94c2d..ce26ff2ff1636 100644 --- a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js +++ b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js @@ -6,29 +6,25 @@ define([ 'ko', 'jquery', + 'rjsResolver', 'Magento_Ui/js/lib/knockout/bindings/color-picker' -], function (ko, $) { +], function (ko, $, resolver) { 'use strict'; - var $input, - originaljQuery, - originaljQueryInit, - originaljQuerySpectrum; - - beforeEach(function () { - originaljQuery = $; - originaljQueryInit = $.fn.init; - originaljQuerySpectrum = $.fn.spectrum; + var $input; + + beforeAll(function () { + define('spectrum', function () { + return jasmine.createSpy(); + }); }); - afterEach(function () { - $ = originaljQuery; - $.fn.init = originaljQueryInit; - $.fn.spectrum = originaljQuerySpectrum; + afterAll(function () { + require.undef('spectrum'); }); describe('Colorpicker binding', function () { - it('Should call spectrum on $input with disabled configuration if view model disabled', function () { + it('Should call spectrum on $input with disabled configuration if view model disabled', function (done) { var value = { configStuffInHere: true }, @@ -41,23 +37,21 @@ define([ $input = jasmine.createSpy(); ko.bindingHandlers.colorPicker.init($input, valueAccessor, null, viewModel); - - expect(value.change).toEqual(jasmine.any(Function)); - expect(value.hide).toEqual(jasmine.any(Function)); - expect(value.show).toEqual(jasmine.any(Function)); - expect(value.change).toBe(value.hide); - - expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['disable']]); - expect(viewModel.disabled).toHaveBeenCalled(); - - $.fn.init = jasmine.createSpy().and.returnValue($.fn); - - ko.bindingHandlers.colorPicker.init($input, valueAccessor, null, viewModel); - - expect($.fn.init).toHaveBeenCalledWith($input, undefined); + + resolver(function () { + expect(value.change).toEqual(jasmine.any(Function)); + expect(value.hide).toEqual(jasmine.any(Function)); + expect(value.show).toEqual(jasmine.any(Function)); + expect(value.change).toBe(value.hide); + + expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['disable']]); + expect(viewModel.disabled).toHaveBeenCalled(); + + done(); + }); }); - it('Should call spectrum on $input with extra configuration if view model enabled', function () { + it('Should call spectrum on $input with extra configuration if view model enabled', function (done) { var value = { configStuffInHere: true }, @@ -71,22 +65,20 @@ define([ ko.bindingHandlers.colorPicker.init($input, valueAccessor, null, viewModel); - expect(value.change).toEqual(jasmine.any(Function)); - expect(value.hide).toEqual(jasmine.any(Function)); - expect(value.show).toEqual(jasmine.any(Function)); - expect(value.change).toBe(value.hide); + resolver(function () { + expect(value.change).toEqual(jasmine.any(Function)); + expect(value.hide).toEqual(jasmine.any(Function)); + expect(value.show).toEqual(jasmine.any(Function)); + expect(value.change).toBe(value.hide); - expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['enable']]); - expect(viewModel.disabled).toHaveBeenCalled(); + expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['enable']]); + expect(viewModel.disabled).toHaveBeenCalled(); - $.fn.init = jasmine.createSpy().and.returnValue($.fn); - - ko.bindingHandlers.colorPicker.init($input, valueAccessor, null, viewModel); - - expect($.fn.init).toHaveBeenCalledWith($input, undefined); + done(); + }); }); - it('Verify config value is empty when reset colorpicker intput', function () { + it('Verify config value is empty when reset colorpicker intput', function (done) { var value = { configStuffInHere: true, value: jasmine.createSpy().and.returnValue(undefined) @@ -100,13 +92,22 @@ define([ $input = jasmine.createSpy(); ko.bindingHandlers.colorPicker.update($input, valueAccessor, null, viewModel); - expect($.fn.spectrum).toHaveBeenCalledTimes(1); - expect(valueAccessor().value).toHaveBeenCalledTimes(4); - value.value = jasmine.createSpy().and.returnValue(''); - ko.bindingHandlers.colorPicker.update($input, valueAccessor, null, viewModel); - expect($.fn.spectrum).toHaveBeenCalledTimes(3); - expect(valueAccessor().value).toHaveBeenCalledTimes(5); + resolver(function () { + expect($.fn.spectrum).toHaveBeenCalledTimes(1); + expect(valueAccessor().value).toHaveBeenCalledTimes(4); + + value.value = jasmine.createSpy().and.returnValue(''); + ko.bindingHandlers.colorPicker.update($input, valueAccessor, null, viewModel); + + resolver(function () { + expect($.fn.spectrum).toHaveBeenCalledTimes(3); + expect(valueAccessor().value).toHaveBeenCalledTimes(5); + + done(); + }); + }); + }); }); }); From 3c56911149694d6574166996bbbf8519b79cb129 Mon Sep 17 00:00:00 2001 From: Mateusz Krzeszowiak Date: Thu, 28 May 2020 09:28:43 +0200 Subject: [PATCH 2/7] Fix static tests --- .../Ui/base/js/lib/ko/bind/color-picker.test.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js index ce26ff2ff1636..cd8041440562b 100644 --- a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js +++ b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js @@ -12,7 +12,7 @@ define([ 'use strict'; var $input; - + beforeAll(function () { define('spectrum', function () { return jasmine.createSpy(); @@ -37,8 +37,8 @@ define([ $input = jasmine.createSpy(); ko.bindingHandlers.colorPicker.init($input, valueAccessor, null, viewModel); - - resolver(function () { + + resolver(function () { //eslint-disable-line max-nested-callbacks expect(value.change).toEqual(jasmine.any(Function)); expect(value.hide).toEqual(jasmine.any(Function)); expect(value.show).toEqual(jasmine.any(Function)); @@ -65,7 +65,7 @@ define([ ko.bindingHandlers.colorPicker.init($input, valueAccessor, null, viewModel); - resolver(function () { + resolver(function () { //eslint-disable-line max-nested-callbacks expect(value.change).toEqual(jasmine.any(Function)); expect(value.hide).toEqual(jasmine.any(Function)); expect(value.show).toEqual(jasmine.any(Function)); @@ -93,21 +93,20 @@ define([ ko.bindingHandlers.colorPicker.update($input, valueAccessor, null, viewModel); - resolver(function () { + resolver(function () { //eslint-disable-line max-nested-callbacks expect($.fn.spectrum).toHaveBeenCalledTimes(1); expect(valueAccessor().value).toHaveBeenCalledTimes(4); value.value = jasmine.createSpy().and.returnValue(''); ko.bindingHandlers.colorPicker.update($input, valueAccessor, null, viewModel); - resolver(function () { + resolver(function () { //eslint-disable-line max-nested-callbacks expect($.fn.spectrum).toHaveBeenCalledTimes(3); expect(valueAccessor().value).toHaveBeenCalledTimes(5); done(); }); }); - }); }); }); From cb58d8626d8b7474a4bdd5bb5b2d9d4e1ded3ae9 Mon Sep 17 00:00:00 2001 From: Mateusz Krzeszowiak Date: Fri, 29 May 2020 09:39:57 +0200 Subject: [PATCH 3/7] Fix static tests --- .../Ui/view/base/web/js/lib/knockout/bindings/color-picker.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js b/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js index b222eec45691c..beea9f5bbbf98 100644 --- a/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js +++ b/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js @@ -52,7 +52,7 @@ define([ return true; }; - require(['tinycolor', 'spectrum'], function() { + require(['tinycolor', 'spectrum'], function () { $(element).spectrum(config); changeColorPickerStateBasedOnViewModel(element, viewModel); @@ -76,7 +76,7 @@ define([ config.value(''); } - require(['tinycolor', 'spectrum'], function(tinycolor) { + require(['tinycolor', 'spectrum'], function (tinycolor) { if (tinycolor(config.value()).isValid() || config.value() === '') { $(element).spectrum('set', config.value()); From ca427a61acb6216e96b3e7d96fe4f6983573f704 Mon Sep 17 00:00:00 2001 From: Mateusz Krzeszowiak Date: Sat, 30 May 2020 18:33:07 +0200 Subject: [PATCH 4/7] Fix tests --- .../js/lib/knockout/bindings/color-picker.js | 2 +- .../base/js/lib/ko/bind/color-picker.test.js | 67 ++++++++++--------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js b/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js index beea9f5bbbf98..a4566481b3d70 100644 --- a/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js +++ b/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/color-picker.js @@ -84,7 +84,7 @@ define([ config.value($(element).spectrum('get').toString()); } } - + changeColorPickerStateBasedOnViewModel(element, viewModel); }); } diff --git a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js index cd8041440562b..312418fa60023 100644 --- a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js +++ b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js @@ -7,6 +7,7 @@ define([ 'ko', 'jquery', 'rjsResolver', + 'tinycolor', 'Magento_Ui/js/lib/knockout/bindings/color-picker' ], function (ko, $, resolver) { 'use strict'; @@ -23,8 +24,16 @@ define([ require.undef('spectrum'); }); + beforeEach(function () { + jasmine.clock().install(); + }); + + afterEach(function () { + jasmine.clock().uninstall(); + }); + describe('Colorpicker binding', function () { - it('Should call spectrum on $input with disabled configuration if view model disabled', function (done) { + it('Should call spectrum on $input with disabled configuration if view model disabled', function () { var value = { configStuffInHere: true }, @@ -38,20 +47,18 @@ define([ ko.bindingHandlers.colorPicker.init($input, valueAccessor, null, viewModel); - resolver(function () { //eslint-disable-line max-nested-callbacks - expect(value.change).toEqual(jasmine.any(Function)); - expect(value.hide).toEqual(jasmine.any(Function)); - expect(value.show).toEqual(jasmine.any(Function)); - expect(value.change).toBe(value.hide); + jasmine.clock().tick(1000); - expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['disable']]); - expect(viewModel.disabled).toHaveBeenCalled(); + expect(value.change).toEqual(jasmine.any(Function)); + expect(value.hide).toEqual(jasmine.any(Function)); + expect(value.show).toEqual(jasmine.any(Function)); + expect(value.change).toBe(value.hide); - done(); - }); + expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['disable']]); + expect(viewModel.disabled).toHaveBeenCalled(); }); - it('Should call spectrum on $input with extra configuration if view model enabled', function (done) { + it('Should call spectrum on $input with extra configuration if view model enabled', function () { var value = { configStuffInHere: true }, @@ -65,20 +72,18 @@ define([ ko.bindingHandlers.colorPicker.init($input, valueAccessor, null, viewModel); - resolver(function () { //eslint-disable-line max-nested-callbacks - expect(value.change).toEqual(jasmine.any(Function)); - expect(value.hide).toEqual(jasmine.any(Function)); - expect(value.show).toEqual(jasmine.any(Function)); - expect(value.change).toBe(value.hide); + jasmine.clock().tick(1000); - expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['enable']]); - expect(viewModel.disabled).toHaveBeenCalled(); + expect(value.change).toEqual(jasmine.any(Function)); + expect(value.hide).toEqual(jasmine.any(Function)); + expect(value.show).toEqual(jasmine.any(Function)); + expect(value.change).toBe(value.hide); - done(); - }); + expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['enable']]); + expect(viewModel.disabled).toHaveBeenCalled(); }); - it('Verify config value is empty when reset colorpicker intput', function (done) { + it('Verify config value is empty when reset colorpicker intput', function () { var value = { configStuffInHere: true, value: jasmine.createSpy().and.returnValue(undefined) @@ -93,20 +98,18 @@ define([ ko.bindingHandlers.colorPicker.update($input, valueAccessor, null, viewModel); - resolver(function () { //eslint-disable-line max-nested-callbacks - expect($.fn.spectrum).toHaveBeenCalledTimes(1); - expect(valueAccessor().value).toHaveBeenCalledTimes(4); + jasmine.clock().tick(1000); + + expect($.fn.spectrum).toHaveBeenCalledTimes(1); + expect(valueAccessor().value).toHaveBeenCalledTimes(4); - value.value = jasmine.createSpy().and.returnValue(''); - ko.bindingHandlers.colorPicker.update($input, valueAccessor, null, viewModel); + value.value = jasmine.createSpy().and.returnValue(''); + ko.bindingHandlers.colorPicker.update($input, valueAccessor, null, viewModel); - resolver(function () { //eslint-disable-line max-nested-callbacks - expect($.fn.spectrum).toHaveBeenCalledTimes(3); - expect(valueAccessor().value).toHaveBeenCalledTimes(5); + jasmine.clock().tick(1000); - done(); - }); - }); + expect($.fn.spectrum).toHaveBeenCalledTimes(3); + expect(valueAccessor().value).toHaveBeenCalledTimes(5); }); }); }); From 95bce39a7d76b02fe8956b4aa5b1e60affa6c660 Mon Sep 17 00:00:00 2001 From: Mateusz Krzeszowiak Date: Sat, 30 May 2020 19:40:51 +0200 Subject: [PATCH 5/7] Remove unused import --- .../code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js index 312418fa60023..28ef517c93ed5 100644 --- a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js +++ b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js @@ -6,10 +6,9 @@ define([ 'ko', 'jquery', - 'rjsResolver', 'tinycolor', 'Magento_Ui/js/lib/knockout/bindings/color-picker' -], function (ko, $, resolver) { +], function (ko, $) { 'use strict'; var $input; From d7c5652931a36dc55e635ba11fd591330b69d957 Mon Sep 17 00:00:00 2001 From: engcom-Echo Date: Wed, 3 Jun 2020 11:52:11 +0300 Subject: [PATCH 6/7] fix tests --- .../base/js/lib/ko/bind/color-picker.test.js | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js index 28ef517c93ed5..1243cd1f8bd5f 100644 --- a/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js +++ b/dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/lib/ko/bind/color-picker.test.js @@ -13,22 +13,14 @@ define([ var $input; - beforeAll(function () { + beforeEach(function () { define('spectrum', function () { return jasmine.createSpy(); }); }); - afterAll(function () { - require.undef('spectrum'); - }); - - beforeEach(function () { - jasmine.clock().install(); - }); - afterEach(function () { - jasmine.clock().uninstall(); + require.undef('spectrum'); }); describe('Colorpicker binding', function () { @@ -41,6 +33,8 @@ define([ disabled: jasmine.createSpy().and.returnValue(true) }; + jasmine.clock().install(); + $.fn.spectrum = jasmine.createSpy(); $input = jasmine.createSpy(); @@ -55,6 +49,8 @@ define([ expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['disable']]); expect(viewModel.disabled).toHaveBeenCalled(); + + jasmine.clock().uninstall(); }); it('Should call spectrum on $input with extra configuration if view model enabled', function () { @@ -66,6 +62,8 @@ define([ disabled: jasmine.createSpy().and.returnValue(false) }; + jasmine.clock().install(); + $.fn.spectrum = jasmine.createSpy(); $input = jasmine.createSpy(); @@ -80,6 +78,8 @@ define([ expect($.fn.spectrum.calls.allArgs()).toEqual([[value], ['enable']]); expect(viewModel.disabled).toHaveBeenCalled(); + + jasmine.clock().uninstall(); }); it('Verify config value is empty when reset colorpicker intput', function () { @@ -92,6 +92,8 @@ define([ disabled: jasmine.createSpy().and.returnValue(false) }; + jasmine.clock().install(); + $.fn.spectrum = jasmine.createSpy(); $input = jasmine.createSpy(); @@ -109,6 +111,8 @@ define([ expect($.fn.spectrum).toHaveBeenCalledTimes(3); expect(valueAccessor().value).toHaveBeenCalledTimes(5); + + jasmine.clock().uninstall(); }); }); }); From 30058c52193c896b5b1b46e8f018992486985ed3 Mon Sep 17 00:00:00 2001 From: "taras.gamanov" Date: Thu, 19 Nov 2020 18:12:54 +0200 Subject: [PATCH 7/7] flaky test has been updated --- .../Mftf/Test/StorefrontAddGroupedProductToShoppingCartTest.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Checkout/Test/Mftf/Test/StorefrontAddGroupedProductToShoppingCartTest.xml b/app/code/Magento/Checkout/Test/Mftf/Test/StorefrontAddGroupedProductToShoppingCartTest.xml index 13a179fe52444..3a7af167b3585 100644 --- a/app/code/Magento/Checkout/Test/Mftf/Test/StorefrontAddGroupedProductToShoppingCartTest.xml +++ b/app/code/Magento/Checkout/Test/Mftf/Test/StorefrontAddGroupedProductToShoppingCartTest.xml @@ -96,7 +96,7 @@ - +