From 8f4198501d43f83a102c811035b6361f9c670fbb Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Tue, 2 Aug 2022 11:08:46 +0700 Subject: [PATCH 1/5] refactor(web): converts test_utils funcs to Promise use --- web/unit_tests/cases/attachmentAPI.js | 24 ++- web/unit_tests/cases/basics.js | 17 +- web/unit_tests/cases/engine.js | 4 +- web/unit_tests/cases/engine_chirality.js | 2 +- web/unit_tests/cases/events.js | 10 +- web/unit_tests/cases/text_selection.js | 8 +- web/unit_tests/test_utils.js | 191 ++++++++--------------- 7 files changed, 97 insertions(+), 159 deletions(-) diff --git a/web/unit_tests/cases/attachmentAPI.js b/web/unit_tests/cases/attachmentAPI.js index e4bc9cd498..a8cf05cf8c 100644 --- a/web/unit_tests/cases/attachmentAPI.js +++ b/web/unit_tests/cases/attachmentAPI.js @@ -8,21 +8,19 @@ describe('Attachment API', function() { fixture.setBase('fixtures'); this.timeout(testconfig.timeouts.scriptLoad * 3); - setupKMW({ attachType:'manual' }, function() { - loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", function() { - // Sequential so we don't have to worry about race conditions and such - // to signal completion with done(). - - loadKeyboardFromJSON("/keyboards/khmer_angkor.json", function() { - keyman.setActiveKeyboard("lao_2008_basic"); - done(); - }, testconfig.timeouts.scriptLoad); - }, testconfig.timeouts.scriptLoad); - }, testconfig.timeouts.scriptLoad); + setupKMW({ attachType:'manual' }, testconfig.timeouts.scriptLoad).then(() => { + const kbd1 = loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", testconfig.timeouts.scriptLoad, { passive: true }); + const kbd2 = loadKeyboardFromJSON("/keyboards/khmer_angkor.json", testconfig.timeouts.scriptLoad, { passive: true }); + Promise.all([kbd1, kbd2]).then(() => { + keyman.setActiveKeyboard("lao_2008_basic"); + done(); + }); + }); }); after(function() { keyman.removeKeyboards('lao_2008_basic'); + keyman.removeKeyboards('khmer_angkor'); teardownKMW(); }); @@ -197,7 +195,7 @@ Modernizr.on('touchevents', function(result) { this.timeout(testconfig.timeouts.scriptLoad); fixture.setBase('fixtures'); - setupKMW({ attachType:'auto' }, done, testconfig.timeouts.scriptLoad); + setupKMW({ attachType:'auto' }, testconfig.timeouts.scriptLoad).then(done); }); beforeEach(function() { @@ -265,7 +263,7 @@ Modernizr.on('touchevents', function(result) { this.timeout(testconfig.timeouts.scriptLoad); fixture.setBase('fixtures'); - setupKMW({ attachType:'auto' }, done, testconfig.timeouts.scriptLoad); + setupKMW({ attachType:'auto' }, testconfig.timeouts.scriptLoad).then(done); }); beforeEach(function() { diff --git a/web/unit_tests/cases/basics.js b/web/unit_tests/cases/basics.js index 621161aa9b..037ff92d8f 100644 --- a/web/unit_tests/cases/basics.js +++ b/web/unit_tests/cases/basics.js @@ -13,7 +13,7 @@ describe('Basic KeymanWeb', function() { fixture.setBase('fixtures'); fixture.load("singleInput.html"); - setupKMW(null, done, testconfig.timeouts.scriptLoad); + setupKMW(null, testconfig.timeouts.scriptLoad).then(done); }); afterEach(function() { @@ -42,8 +42,8 @@ Modernizr.on('touchevents', function(result) { fixture.setBase('fixtures'); fixture.load('singleInput.html'); - // Sequentially loads two scripts, so 2x timeout. - setupKMW('toggle', done, testconfig.timeouts.uiLoad, function() { return keyman.ui.initialized; }); + // Loads two scripts in parallel, but just in case, 2x timeout. + setupKMW('toggle', testconfig.timeouts.uiLoad).then(done); }); afterEach(function() { @@ -76,8 +76,8 @@ Modernizr.on('touchevents', function(result) { fixture.setBase('fixtures'); fixture.load('singleInput.html'); - // Sequentially loads two scripts, so 2x timeout. - setupKMW('button', done, testconfig.timeouts.uiLoad, function() { return keyman.ui.init; }); + // Loads two scripts in parallel, but just in case, 2x timeout. + setupKMW('button', testconfig.timeouts.uiLoad).then(done); }); afterEach(function() { @@ -97,8 +97,8 @@ Modernizr.on('touchevents', function(result) { fixture.setBase('fixtures'); fixture.load('singleInput.html'); - // Sequentially loads two scripts, so 2x timeout. - setupKMW('float', done, testconfig.timeouts.uiLoad, function() { return keyman.ui.initialized; }); + // Loads two scripts in parallel, but just in case, 2x timeout. + setupKMW('float', testconfig.timeouts.uiLoad).then(done); }); afterEach(function() { @@ -131,7 +131,8 @@ Modernizr.on('touchevents', function(result) { fixture.setBase('fixtures'); fixture.load('singleInput.html'); - setupKMW('toolbar', done, testconfig.timeouts.uiLoad, function() { return keyman.ui.init; }); + // Loads two scripts in parallel, but just in case, 2x timeout. + setupKMW('toolbar', testconfig.timeouts.uiLoad).then(done); }); afterEach(function() { diff --git a/web/unit_tests/cases/engine.js b/web/unit_tests/cases/engine.js index dbc8e69a32..b1c606af99 100644 --- a/web/unit_tests/cases/engine.js +++ b/web/unit_tests/cases/engine.js @@ -5,7 +5,7 @@ describe('Engine - Browser Interactions', function() { before(function(done) { fixture.setBase('fixtures'); - setupKMW(null, done, testconfig.timeouts.scriptLoad); + setupKMW(null, testconfig.timeouts.scriptLoad).then(done); }); beforeEach(function(done) { @@ -215,7 +215,7 @@ describe('Unmatched Final Groups', function() { before(function(done) { fixture.setBase('fixtures'); - setupKMW(null, done, testconfig.timeouts.scriptLoad + testconfig.timeouts.eventDelay); + setupKMW(null, testconfig.timeouts.scriptLoad + testconfig.timeouts.eventDelay).then(done); }); beforeEach(function(done) { diff --git a/web/unit_tests/cases/engine_chirality.js b/web/unit_tests/cases/engine_chirality.js index 80ebcce0c3..59210bcf26 100644 --- a/web/unit_tests/cases/engine_chirality.js +++ b/web/unit_tests/cases/engine_chirality.js @@ -5,7 +5,7 @@ describe('Engine - Chirality', function() { before(function(done) { fixture.setBase('fixtures'); - setupKMW(null, done, testconfig.timeouts.scriptLoad); + setupKMW(null, testconfig.timeouts.scriptLoad).then(done); }); beforeEach(function(done) { diff --git a/web/unit_tests/cases/events.js b/web/unit_tests/cases/events.js index f26003620e..3fd1f6fde8 100644 --- a/web/unit_tests/cases/events.js +++ b/web/unit_tests/cases/events.js @@ -8,15 +8,11 @@ describe('Event Management', function() { fixture.setBase('fixtures'); fixture.load("eventTestConfig.html"); - setupKMW(null, function() { + setupKMW(null, testconfig.timeouts.scriptLoad).then(() => { // We use this keyboard since we only need minimal input functionality for these tests. // Smaller is better when dealing with net latency. - loadKeyboardFromJSON("/keyboards/test_simple_deadkeys.json", function() { - // Interestingly, when auto-testing there's a Safari bug that prevents - // this from being preserved after the first forced blur command below. - done(); - }, testconfig.timeouts.scriptLoad); - }, testconfig.timeouts.scriptLoad); + loadKeyboardFromJSON("/keyboards/test_simple_deadkeys.json", testconfig.timeouts.scriptLoad).then(done); + }); }); diff --git a/web/unit_tests/cases/text_selection.js b/web/unit_tests/cases/text_selection.js index bc5f31ba92..5fef14a249 100644 --- a/web/unit_tests/cases/text_selection.js +++ b/web/unit_tests/cases/text_selection.js @@ -83,12 +83,12 @@ describe('Text Selection', function() { fixture.load("single"+inputType+".html"); this.timeout(testconfig.timeouts.scriptLoad*2); - setupKMW(null, function() { - loadKeyboardFromJSON("/keyboards/web_context_tests.json", function() { + setupKMW(null, testconfig.timeouts.scriptLoad).then(() => { + loadKeyboardFromJSON("/keyboards/web_context_tests.json", testconfig.timeouts.scriptLoad).then(() => { keyman.setActiveKeyboard("web_context_tests"); done(); - }, testconfig.timeouts.scriptLoad); - }, testconfig.timeouts.scriptLoad); + }); + }); }); after(function() { diff --git a/web/unit_tests/test_utils.js b/web/unit_tests/test_utils.js index 06e98bc316..c2eacfff73 100644 --- a/web/unit_tests/test_utils.js +++ b/web/unit_tests/test_utils.js @@ -16,7 +16,7 @@ try { // Keyman test suite utility methods -var setupKMW = function(kmwOptions, done, timeout, uiInitCheck) { +var setupKMW = function(kmwOptions, timeout) { var ui; if(typeof(kmwOptions) == 'string' || typeof(kmwOptions) == 'undefined' || kmwOptions == null) { @@ -33,8 +33,9 @@ var setupKMW = function(kmwOptions, done, timeout, uiInitCheck) { } } - var kmw = setupScript('source/keymanweb.js', done, timeout, uiInitCheck); - fixture.el.appendChild(kmw); + const kmwPromise = setupScript('source/keymanweb.js', timeout, (scriptEle) => { + fixture.el.appendChild(scriptEle); + }); ui = kmwOptions.ui; @@ -48,43 +49,68 @@ var setupKMW = function(kmwOptions, done, timeout, uiInitCheck) { kmwOptions.resources = '../../../../source'; } + let uiPromise; if(ui) { - var ui = setupScript('source/kmwui' + ui + '.js'); - fixture.el.appendChild(ui); + uiPromise = setupScript('source/kmwui' + ui + '.js', timeout, (scriptEle) => { + fixture.el.appendChild(scriptEle); + }); kmwOptions.ui=ui; } - var initFunc = function() { - if(window['keyman']) { - window['keyman'].init(kmwOptions); - } else { - window.setTimeout(function() { - initFunc(); - }) - } - }; - - /* Keep this timeout short - if set too long, kmwinit's default initialization - * will kick in and prevent our settings from going through! - */ - window.setTimeout(function() { - initFunc(); - }, 5); -} - -var setupScript = function(src, done, timeout, uiInitCheck) { - var Lscript = document.createElement('script'); - Lscript.charset="UTF-8"; // KMEW-89 - Lscript.type = 'text/javascript'; - Lscript.async = false; - if(done) { - Lscript.onload = initTimer(done, timeout, uiInitCheck); + let compositePromise = kmwPromise; + if(uiPromise) { + compositePromise = Promise.all([kmwPromise, uiPromise]); } - Lscript.src = src; + return finalPromise = compositePromise.then(() => { + if(window['keyman']) { + return window['keyman'].init(kmwOptions); + } else { + return Promise.reject(); + } + }); +} - return Lscript; +/** + * Produces a script element tied to a Promise for its eventual load (or failure thereof). + * + * The script element is only available via callback due to implementation constraints. + * + * @param {*} src The source script's (relative) path on the test server. + * @param {*} timeout + * @param {*} functor A callback to handle the script element. + * @returns + */ +var setupScript = function(src, timeout, functor) { + return new Promise((resolve, reject) => { + const Lscript = document.createElement('script'); + let hasResolved = false; + Lscript.charset="UTF-8"; // KMEW-89 + Lscript.type = 'text/javascript'; + Lscript.async = false; + + const timer = window.setTimeout(() => { + reject(); + }, timeout); + + Lscript.onload = Lscript.onreadystatechange = () => { + window.clearTimeout(timer); + if(!hasResolved && (Lscript.readyState === undefined || Lscript.readyState == "complete")) { + hasResolved = true; + resolve(); + } + } + + Lscript.onerror = () => { + window.clearTimeout(timer); + reject(); + } + + Lscript.src = src; + + functor(Lscript); + }); } var teardownKMW = function() { @@ -113,90 +139,7 @@ var teardownKMW = function() { } } -// Make sure the main script loads... -var initTimer = function(done, timeout, uiInitCheck) { - var uiLoadDelay; - if(typeof(uiInitCheck) != 'function') { - uiInitCheck = function() { return true; }; - uiLoadDelay = false; - } else { - uiLoadDelay = true; - } - - // We need managed state for this. - var InitializationManager = function() { - this.killSwitch = false; - - this.initCheckCallback = function() { - if(window['keyman'] && window['keyman'].initialized == 2 && uiInitCheck()) { - if(done) { - this.timer = window.setTimeout(function() { - // There can be some odd cross-interference with the UI modules and their initialization. - // We use a significant delay here to avoid said problems. - done(); - }, uiLoadDelay ? 2000 : 0); - } - } else if(!this.killSwitch) { - this.timer = window.setTimeout(this.initCheckCallback, 50); - } - }.bind(this); - - if(timeout) { - window.setTimeout(function() { - this.killSwitch = true; - - if(this.timer) { - window.clearTimeout(this.timer); - this.timer = 0; - } - }.bind(this), timeout); - } - } - - var im = new InitializationManager(); - return im.initCheckCallback; -}; - -// Make sure the main script loads... -var onScriptLoad = function(scriptURL, callback, timeout) { - var ScriptLoadObserver = function() { - this.target = document.createElement('a'); - this.target.href = scriptURL; - - if(timeout) { - this.timer = window.setTimeout(function() { - if(this.mo) { - this.mo.disconnect(); - } - }.bind(this), timeout); - } - - var moCallback = function(mutations) { - for(var i=0; i < mutations.length; i++) { - var mutation = mutations[i]; - for(var j=0; j < mutation.addedNodes.length; j++) { - var child = mutation.addedNodes[j]; - if(child instanceof HTMLScriptElement) { - if(child.src == this.target.href) { - child.onload = callback; - } - } - } - } - } - - this.observe = function() { - var config = { childList: true, subtree: true }; - this.mo = new MutationObserver(moCallback.bind(this)); - this.mo.observe(document, config); - } - } - - var slo = new ScriptLoadObserver(); - slo.observe(); -}; - -var loadKeyboardStub = function(stub, callback, timeout, params) { +var loadKeyboardStub = function(stub, timeout, params) { var kbdName = "Keyboard_" + stub.id; keyman.addKeyboards(stub); @@ -205,18 +148,18 @@ var loadKeyboardStub = function(stub, callback, timeout, params) { } if(keyman.getActiveKeyboard() != kbdName) { - onScriptLoad(stub.filename, function() { - callback(); - }, timeout); + return setupScript(stub.filename, timeout, (ele) =>{ + fixture.el.appendChild(ele); + }); } else { - callback(); + return Promise.resolve(); } } -var loadKeyboardFromJSON = function(jsonPath, callback, timeout) { +var loadKeyboardFromJSON = function(jsonPath, timeout, params) { var stub = fixture.load(jsonPath, true); - loadKeyboardStub(stub, callback, timeout); + return loadKeyboardStub(stub, timeout, params); } function runLoadedKeyboardTest(testDef, device, usingOSK, assertCallback) { @@ -234,11 +177,11 @@ function runKeyboardTestFromJSON(jsonPath, params, callback, assertCallback, tim let device = new com.keyman.Device(); device.detect(); - loadKeyboardStub(testSpec.keyboard, function() { + loadKeyboardStub(testSpec.keyboard, timeout).then(() => { runLoadedKeyboardTest(testSpec, device.coreSpec, params.usingOSK, assertCallback); keyman.removeKeyboards(testSpec.keyboard.id); callback(); - }, timeout); + }); } function retrieveAndReset(Pelem) { From d2a92494a5ecce6b624acbb243a5fd9c3f79b18b Mon Sep 17 00:00:00 2001 From: jahorton Date: Tue, 2 Aug 2022 15:36:34 +0700 Subject: [PATCH 2/5] fix(web): Promise logic for keyboard-loading tests --- web/unit_tests/cases/engine.js | 97 +++++++++++++++++++++++----------- web/unit_tests/test_utils.js | 14 +++-- 2 files changed, 72 insertions(+), 39 deletions(-) diff --git a/web/unit_tests/cases/engine.js b/web/unit_tests/cases/engine.js index b1c606af99..5cf772ba71 100644 --- a/web/unit_tests/cases/engine.js +++ b/web/unit_tests/cases/engine.js @@ -3,9 +3,9 @@ var assert = chai.assert; describe('Engine - Browser Interactions', function() { this.timeout(testconfig.timeouts.scriptLoad); - before(function(done) { + before(function() { fixture.setBase('fixtures'); - setupKMW(null, testconfig.timeouts.scriptLoad).then(done); + return setupKMW(null, testconfig.timeouts.scriptLoad); }); beforeEach(function(done) { @@ -24,24 +24,8 @@ describe('Engine - Browser Interactions', function() { fixture.cleanup(); }); - describe('Keyboard Loading', function() { - it('Local', function(done) { - this.timeout(testconfig.timeouts.scriptLoad); - - var test_callback = function() { - assert.isNotNull(keyman.getKeyboard("lao_2008_basic", "lo"), "Keyboard stub was not registered!"); - assert.equal(keyman.getActiveKeyboard(), "Keyboard_lao_2008_basic", "Keyboard not set automatically!"); - keyman.removeKeyboards('lao_2008_basic'); - assert.equal(keyman.getActiveKeyboard(), '', "Keyboard not removed correctly!"); - done(); - } - - loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", test_callback, testconfig.timeouts.scriptLoad, {passive: true}); - }); - }); - describe('RegisterStub', function() { - it.skip('RegisterStub on same keyboard twice', function(done) { + it.skip('RegisterStub on same keyboard twice', function() { // mcdurdin: skipping this test for now as it is sporadically failing and have not been able to trace source of issue // see https://github.com/keymanapp/keyman/issues/5799 this.timeout(testconfig.timeouts.scriptLoad); @@ -51,10 +35,10 @@ describe('Engine - Browser Interactions', function() { assert.equal(keyman.getActiveKeyboard(), "Keyboard_lao_2008_basic", "Keyboard not set automatically!"); keyman.removeKeyboards('lao_2008_basic'); assert.equal(keyman.getActiveKeyboard(), '', "Keyboard not removed correctly!"); - done(); } - loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", test_callback, testconfig.timeouts.scriptLoad, {passive: true}); + let finalPromise = loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", testconfig.timeouts.scriptLoad, {passive: true}) + .then(test_callback); var stub = { 'KI': 'Keyboard_lao_2008_basic', @@ -64,7 +48,7 @@ describe('Engine - Browser Interactions', function() { 'KF': 'resources/keyboards/lao_2008_basic.js' }; assert.equal(com.keyman.text.KeyboardInterface.prototype.registerStub(stub), 1, "Registering existing keyboard should return 1!"); - done(); + return finalPromise; }); }); @@ -72,8 +56,8 @@ describe('Engine - Browser Interactions', function() { describe('Variable Stores', function() { this.timeout(testconfig.timeouts.scriptLoad + testconfig.timeouts.standard); - beforeEach(function(done) { - loadKeyboardFromJSON("/keyboards/options_with_save.json", done, testconfig.timeouts.scriptLoad); + beforeEach(function() { + return loadKeyboardFromJSON("/keyboards/options_with_save.json", testconfig.timeouts.scriptLoad); }); after(function() { @@ -110,9 +94,10 @@ describe('Engine - Browser Interactions', function() { done(); } - loadKeyboardFromJSON("/keyboards/options_with_save.json", function() { - keyman.setActiveKeyboard(keyboardID, 'en').then(remainderOfTest); - }, testconfig.timeouts.scriptLoad); + return loadKeyboardFromJSON("/keyboards/options_with_save.json", testconfig.timeouts.scriptLoad) + .then(() => { + return keyman.setActiveKeyboard(keyboardID, 'en'); + }).then(remainderOfTest); }); }); @@ -144,9 +129,9 @@ describe('Engine - Browser Interactions', function() { describe('Integrated Simulation Checks', function() { this.timeout(testconfig.timeouts.standard); - before(function(done){ + before(function() { this.timeout = testconfig.timeouts.scriptLoad; - loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", done, testconfig.timeouts.scriptLoad); + return loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", testconfig.timeouts.scriptLoad); }); beforeEach(function() { @@ -213,9 +198,9 @@ describe('Engine - Browser Interactions', function() { describe('Unmatched Final Groups', function() { this.timeout(testconfig.timeouts.scriptLoad); - before(function(done) { + before(function() { fixture.setBase('fixtures'); - setupKMW(null, testconfig.timeouts.scriptLoad + testconfig.timeouts.eventDelay).then(done); + return setupKMW(null, testconfig.timeouts.scriptLoad + testconfig.timeouts.eventDelay); }); beforeEach(function(done) { @@ -240,3 +225,53 @@ describe('Unmatched Final Groups', function() { runKeyboardTestFromJSON('/engine_tests/ghp_enter.json', {usingOSK: true}, done, assert.equal, testconfig.timeouts.scriptLoad); }); }); + +// Kept separate to maintain an extra-clean setup for this test. +describe('Engine - Browser Interactions', function() { + this.timeout(testconfig.timeouts.scriptLoad); + + before(function() { + fixture.setBase('fixtures'); + }); + + beforeEach(function() { + fixture.load("singleInput.html"); + return setupKMW(null, testconfig.timeouts.scriptLoad); + }); + + afterEach(function() { + fixture.cleanup(); + teardownKMW(); + }); + + describe('Keyboard Loading', function() { + it('Local', function() { + this.timeout(testconfig.timeouts.scriptLoad); + + var test_callback = function() { + assert.isNotNull(keyman.getKeyboard("lao_2008_basic", "lo"), "Keyboard stub was not registered!"); + assert.equal(keyman.getActiveKeyboard(), "Keyboard_lao_2008_basic", "Keyboard not set automatically!"); + keyman.removeKeyboards('lao_2008_basic'); + assert.equal(keyman.getActiveKeyboard(), '', "Keyboard not removed correctly!"); + } + + return loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", testconfig.timeouts.scriptLoad) + .then(test_callback); + }); + + it('Automatically sets first available keyboard', function() { + this.timeout(testconfig.timeouts.scriptLoad); + + var test_callback = function() { + assert.isNotNull(keyman.getKeyboard("lao_2008_basic", "lo"), "Keyboard stub was not registered!"); + assert.equal(keyman.getActiveKeyboard(), "Keyboard_lao_2008_basic", "Keyboard not set automatically!"); + keyman.removeKeyboards('lao_2008_basic'); + assert.equal(keyman.getActiveKeyboard(), '', "Keyboard not removed correctly!"); + } + + // Still acting flaky, though... Hmm. + return loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", testconfig.timeouts.scriptLoad, {passive: true}) + .then(test_callback); + }); + }); +}); \ No newline at end of file diff --git a/web/unit_tests/test_utils.js b/web/unit_tests/test_utils.js index c2eacfff73..b20a52773e 100644 --- a/web/unit_tests/test_utils.js +++ b/web/unit_tests/test_utils.js @@ -91,7 +91,7 @@ var setupScript = function(src, timeout, functor) { Lscript.async = false; const timer = window.setTimeout(() => { - reject(); + reject("Script load attempt timed out."); }, timeout); Lscript.onload = Lscript.onreadystatechange = () => { @@ -102,9 +102,9 @@ var setupScript = function(src, timeout, functor) { } } - Lscript.onerror = () => { + Lscript.onerror = (err) => { window.clearTimeout(timer); - reject(); + reject(err); } Lscript.src = src; @@ -144,11 +144,9 @@ var loadKeyboardStub = function(stub, timeout, params) { keyman.addKeyboards(stub); if(!params || !params.passive) { - keyman.setActiveKeyboard(kbdName, stub.languages.id); - } - - if(keyman.getActiveKeyboard() != kbdName) { - return setupScript(stub.filename, timeout, (ele) =>{ + return keyman.setActiveKeyboard(kbdName, stub.languages.id); + } else if(keyman.getActiveKeyboard() != kbdName) { + return setupScript(stub.filename, timeout, (ele) => { fixture.el.appendChild(ele); }); } else { From 967e36e405ca761ce3b4f747032100497ef89734 Mon Sep 17 00:00:00 2001 From: jahorton Date: Tue, 2 Aug 2022 15:42:51 +0700 Subject: [PATCH 3/5] chore(web): cleaner promise flow for other tests --- web/unit_tests/cases/attachmentAPI.js | 17 ++++++++--------- web/unit_tests/cases/basics.js | 20 ++++++++++---------- web/unit_tests/cases/engine_chirality.js | 4 ++-- web/unit_tests/cases/events.js | 7 +++---- web/unit_tests/cases/text_selection.js | 9 ++++----- 5 files changed, 27 insertions(+), 30 deletions(-) diff --git a/web/unit_tests/cases/attachmentAPI.js b/web/unit_tests/cases/attachmentAPI.js index a8cf05cf8c..488761b932 100644 --- a/web/unit_tests/cases/attachmentAPI.js +++ b/web/unit_tests/cases/attachmentAPI.js @@ -3,17 +3,16 @@ var assert = chai.assert; describe('Attachment API', function() { this.timeout(testconfig.timeouts.standard); - before(function(done) { + before(function() { assert.isFalse(com.keyman.karma.DEVICE_DETECT_FAILURE, "Cannot run due to device detection failure."); fixture.setBase('fixtures'); this.timeout(testconfig.timeouts.scriptLoad * 3); - setupKMW({ attachType:'manual' }, testconfig.timeouts.scriptLoad).then(() => { + return setupKMW({ attachType:'manual' }, testconfig.timeouts.scriptLoad).then(() => { const kbd1 = loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", testconfig.timeouts.scriptLoad, { passive: true }); const kbd2 = loadKeyboardFromJSON("/keyboards/khmer_angkor.json", testconfig.timeouts.scriptLoad, { passive: true }); - Promise.all([kbd1, kbd2]).then(() => { - keyman.setActiveKeyboard("lao_2008_basic"); - done(); + return Promise.all([kbd1, kbd2]).then(() => { + return keyman.setActiveKeyboard("lao_2008_basic"); }); }); }); @@ -191,11 +190,11 @@ Modernizr.on('touchevents', function(result) { this.timeout(testconfig.timeouts.standard); - before(function(done) { + before(function() { this.timeout(testconfig.timeouts.scriptLoad); fixture.setBase('fixtures'); - setupKMW({ attachType:'auto' }, testconfig.timeouts.scriptLoad).then(done); + return setupKMW({ attachType:'auto' }, testconfig.timeouts.scriptLoad); }); beforeEach(function() { @@ -259,11 +258,11 @@ Modernizr.on('touchevents', function(result) { this.timeout(testconfig.timeouts.standard); - before(function(done) { + before(function() { this.timeout(testconfig.timeouts.scriptLoad); fixture.setBase('fixtures'); - setupKMW({ attachType:'auto' }, testconfig.timeouts.scriptLoad).then(done); + return setupKMW({ attachType:'auto' }, testconfig.timeouts.scriptLoad); }); beforeEach(function() { diff --git a/web/unit_tests/cases/basics.js b/web/unit_tests/cases/basics.js index 037ff92d8f..c3d77779bb 100644 --- a/web/unit_tests/cases/basics.js +++ b/web/unit_tests/cases/basics.js @@ -8,12 +8,12 @@ describe('Basic KeymanWeb', function() { assert.isFalse(com.keyman.karma.DEVICE_DETECT_FAILURE, "Cannot run due to device detection failure."); }) - beforeEach(function(done) { + beforeEach(function() { this.timeout(testconfig.timeouts.scriptLoad); fixture.setBase('fixtures'); fixture.load("singleInput.html"); - setupKMW(null, testconfig.timeouts.scriptLoad).then(done); + return setupKMW(null, testconfig.timeouts.scriptLoad); }); afterEach(function() { @@ -37,13 +37,13 @@ Modernizr.on('touchevents', function(result) { describe('Basic Toggle UI', function() { this.timeout(testconfig.timeouts.scriptLoad); - beforeEach(function(done) { + beforeEach(function() { this.timeout(testconfig.timeouts.uiLoad); fixture.setBase('fixtures'); fixture.load('singleInput.html'); // Loads two scripts in parallel, but just in case, 2x timeout. - setupKMW('toggle', testconfig.timeouts.uiLoad).then(done); + return setupKMW('toggle', testconfig.timeouts.uiLoad); }); afterEach(function() { @@ -71,13 +71,13 @@ Modernizr.on('touchevents', function(result) { describe('Basic Button UI', function() { - beforeEach(function(done) { + beforeEach(function() { this.timeout(testconfig.timeouts.uiLoad); fixture.setBase('fixtures'); fixture.load('singleInput.html'); // Loads two scripts in parallel, but just in case, 2x timeout. - setupKMW('button', testconfig.timeouts.uiLoad).then(done); + return setupKMW('button', testconfig.timeouts.uiLoad); }); afterEach(function() { @@ -92,13 +92,13 @@ Modernizr.on('touchevents', function(result) { describe('Basic Float UI', function() { - beforeEach(function(done) { + beforeEach(function() { this.timeout(testconfig.timeouts.uiLoad); fixture.setBase('fixtures'); fixture.load('singleInput.html'); // Loads two scripts in parallel, but just in case, 2x timeout. - setupKMW('float', testconfig.timeouts.uiLoad).then(done); + return setupKMW('float', testconfig.timeouts.uiLoad); }); afterEach(function() { @@ -126,13 +126,13 @@ Modernizr.on('touchevents', function(result) { describe('Basic Toolbar UI', function() { - beforeEach(function(done) { + beforeEach(function() { this.timeout(testconfig.timeouts.uiLoad); fixture.setBase('fixtures'); fixture.load('singleInput.html'); // Loads two scripts in parallel, but just in case, 2x timeout. - setupKMW('toolbar', testconfig.timeouts.uiLoad).then(done); + return setupKMW('toolbar', testconfig.timeouts.uiLoad); }); afterEach(function() { diff --git a/web/unit_tests/cases/engine_chirality.js b/web/unit_tests/cases/engine_chirality.js index 59210bcf26..fa186f8146 100644 --- a/web/unit_tests/cases/engine_chirality.js +++ b/web/unit_tests/cases/engine_chirality.js @@ -3,9 +3,9 @@ var assert = chai.assert; describe('Engine - Chirality', function() { this.timeout(testconfig.timeouts.scriptLoad); - before(function(done) { + before(function() { fixture.setBase('fixtures'); - setupKMW(null, testconfig.timeouts.scriptLoad).then(done); + return setupKMW(null, testconfig.timeouts.scriptLoad); }); beforeEach(function(done) { diff --git a/web/unit_tests/cases/events.js b/web/unit_tests/cases/events.js index 3fd1f6fde8..e32ad27dfe 100644 --- a/web/unit_tests/cases/events.js +++ b/web/unit_tests/cases/events.js @@ -3,17 +3,16 @@ var assert = chai.assert; describe('Event Management', function() { this.timeout(testconfig.timeouts.standard); - before(function(done) { + before(function() { this.timeout(testconfig.timeouts.scriptLoad * 2); fixture.setBase('fixtures'); fixture.load("eventTestConfig.html"); - setupKMW(null, testconfig.timeouts.scriptLoad).then(() => { + return setupKMW(null, testconfig.timeouts.scriptLoad).then(() => { // We use this keyboard since we only need minimal input functionality for these tests. // Smaller is better when dealing with net latency. - loadKeyboardFromJSON("/keyboards/test_simple_deadkeys.json", testconfig.timeouts.scriptLoad).then(done); + return loadKeyboardFromJSON("/keyboards/test_simple_deadkeys.json", testconfig.timeouts.scriptLoad); }); - }); after(function() { diff --git a/web/unit_tests/cases/text_selection.js b/web/unit_tests/cases/text_selection.js index 5fef14a249..33ad0061e4 100644 --- a/web/unit_tests/cases/text_selection.js +++ b/web/unit_tests/cases/text_selection.js @@ -76,17 +76,16 @@ describe('Text Selection', function() { // TODO: Add automated tests for editable DIV, designMode iframe for(var inputType of ['Input', 'TextArea']) { describe('Text Selection in '+inputType, function() { - before(function(done) { + before(function() { // These tests require use of KMW's device-detection functionality. assert.isFalse(com.keyman.karma.DEVICE_DETECT_FAILURE, "Cannot run due to device detection failure."); fixture.setBase('fixtures'); fixture.load("single"+inputType+".html"); this.timeout(testconfig.timeouts.scriptLoad*2); - setupKMW(null, testconfig.timeouts.scriptLoad).then(() => { - loadKeyboardFromJSON("/keyboards/web_context_tests.json", testconfig.timeouts.scriptLoad).then(() => { - keyman.setActiveKeyboard("web_context_tests"); - done(); + return setupKMW(null, testconfig.timeouts.scriptLoad).then(() => { + return loadKeyboardFromJSON("/keyboards/web_context_tests.json", testconfig.timeouts.scriptLoad).then(() => { + return keyman.setActiveKeyboard("web_context_tests"); }); }); }); From 8fab028632f275bf3c1deac23b00cc8f759dfd43 Mon Sep 17 00:00:00 2001 From: jahorton Date: Tue, 2 Aug 2022 15:47:28 +0700 Subject: [PATCH 4/5] chore(web): minor unit test issue review, tweak --- web/unit_tests/cases/engine.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/web/unit_tests/cases/engine.js b/web/unit_tests/cases/engine.js index 5cf772ba71..5ac13b79d6 100644 --- a/web/unit_tests/cases/engine.js +++ b/web/unit_tests/cases/engine.js @@ -25,9 +25,7 @@ describe('Engine - Browser Interactions', function() { }); describe('RegisterStub', function() { - it.skip('RegisterStub on same keyboard twice', function() { - // mcdurdin: skipping this test for now as it is sporadically failing and have not been able to trace source of issue - // see https://github.com/keymanapp/keyman/issues/5799 + it('RegisterStub on same keyboard twice', function() { this.timeout(testconfig.timeouts.scriptLoad); var test_callback = function() { @@ -37,7 +35,7 @@ describe('Engine - Browser Interactions', function() { assert.equal(keyman.getActiveKeyboard(), '', "Keyboard not removed correctly!"); } - let finalPromise = loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", testconfig.timeouts.scriptLoad, {passive: true}) + let finalPromise = loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", testconfig.timeouts.scriptLoad) .then(test_callback); var stub = { @@ -259,7 +257,10 @@ describe('Engine - Browser Interactions', function() { .then(test_callback); }); - it('Automatically sets first available keyboard', function() { + it.skip('Automatically sets first available keyboard', function() { + // While not the original test mentioned by https://github.com/keymanapp/keyman/issues/5799, + // it's showing that same instability now. It appears that the lazy-init code for the first + // added keyboard stub isn't working consistently. this.timeout(testconfig.timeouts.scriptLoad); var test_callback = function() { From 4d14bc6998dbaa2d26dbd016e57af3fd129157a4 Mon Sep 17 00:00:00 2001 From: jahorton Date: Tue, 2 Aug 2022 16:10:04 +0700 Subject: [PATCH 5/5] fix(web): fixes previously-unstable unit test --- web/unit_tests/cases/engine.js | 40 ++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/web/unit_tests/cases/engine.js b/web/unit_tests/cases/engine.js index 5ac13b79d6..87caf0b454 100644 --- a/web/unit_tests/cases/engine.js +++ b/web/unit_tests/cases/engine.js @@ -257,11 +257,8 @@ describe('Engine - Browser Interactions', function() { .then(test_callback); }); - it.skip('Automatically sets first available keyboard', function() { - // While not the original test mentioned by https://github.com/keymanapp/keyman/issues/5799, - // it's showing that same instability now. It appears that the lazy-init code for the first - // added keyboard stub isn't working consistently. - this.timeout(testconfig.timeouts.scriptLoad); + it('Automatically sets first available keyboard', function() { + this.timeout(2 * testconfig.timeouts.scriptLoad); var test_callback = function() { assert.isNotNull(keyman.getKeyboard("lao_2008_basic", "lo"), "Keyboard stub was not registered!"); @@ -270,9 +267,38 @@ describe('Engine - Browser Interactions', function() { assert.equal(keyman.getActiveKeyboard(), '', "Keyboard not removed correctly!"); } - // Still acting flaky, though... Hmm. return loadKeyboardFromJSON("/keyboards/lao_2008_basic.json", testconfig.timeouts.scriptLoad, {passive: true}) - .then(test_callback); + .then(() => { + // Because we're loading the keyboard 'passively', KMW's setActiveKeyboard function is auto-called + // on the stub-add. That specific call (for first keyboard auto-activation) is outside of KMW's + // current Promise chain, so we can't _directly_ rely on a KMW Promise to test it. + return new Promise((resolve) => { + let hasResolved = false; + // So, we give KMW the time needed for auto-activation to happen, polling a bit actively so that we don't + // wait unnecessarily long after it occurs. + let absoluteTimer = window.setTimeout(() => { + if(!hasResolved) { + resolve(); + hasResolved = true; + } + + window.clearTimeout(intervalTimer); + }, testconfig.timeouts.scriptLoad); + + let intervalTimer = window.setInterval(() => { + if(keyman.getActiveKeyboard() != '') { + window.clearTimeout(intervalTimer); + window.clearTimeout(absoluteTimer); + + if(!hasResolved) { + resolve(); + hasResolved = true; + } + } + }, 50); + }); + // Once this delay-Promise resolves successfully (either way)... + }).then(test_callback); // THEN we run our checks. }); }); }); \ No newline at end of file