[web] KeymanWeb - Engine testing #585

jahorton · 2018-02-12T03:27:12Z

Possible enhancements to be desired:

Generate a custom error message (with fail() signal, of course) that collects all errors for a KeyboardTest instead of instantly failing on the first mismatched sequence when running the tests.

This PR serves to introduce and facilitate automated testing of the core KMW keystroke/input processing system, providing two modules toward this end:

Engine Tester

The new file engine.js in the test case section implements the automated testing functionality, first running a few basic tests to ensure individual keystroke and keyboard loading correctness. The real meat is toward the end of the file, demonstrating how to automate an input test sequence through KMW.

The existing test isn't terribly extensive, but serves as a proof-of-concept we can expand upon greatly in order to address various keyboard issues into the future.

Engine Recorder

To facilitate development of engine-related test cases, I've developed a few scripts and a web page designed to record KMW input on any device and generate corresponding JSON consumable by the Engine Tester. It'll take a bit of copy-paste and external file management, but the page takes care of writing the tedious aspects of the JSON content needed for any test. Heck, it can even load saved JSON and extend its data; the main issue is that JavaScript is typically blocked from saving anything to the user's filesystem.

Accordingly, the file structure of the testing area has expanded again, with a section devoted to JSON file content that can be preloaded by the Karma system for each webpage (hence targeting the JSON format for engine test specs).

Furthermore, a few changes to the OSK module of KMW were necessary in order to properly integrate the engine tester and the engine recorder - each had separate issues. Also, the Device object's definition was refactored out of kmwutils.js into its own file, allowing KMW's device-detection code to be utilized by the recorder.

…est.

…ered from touch tests.

…gine testing.

…testable. Problem: touch events.

…s in Chrome emulation.

…tub property.

…engine' test case.

… build into the test command.

…run.

mcdurdin

Looks good, although it's a fairly big set of changes so I really would like to run some tests on my machine before approval. I am in the middle of some code here so will do testing after you address my comments on the PR.

There are a few minor tweaks and one thing I think is a bug. I didn't spend as long on the recorder code as I did on the KMW base changes.

mcdurdin · 2018-02-15T00:34:10Z

web/source/build_recorder.sh

@@ -0,0 +1,54 @@
+#! /bin/bash
+# 
+# Compile keymanweb and copy compiled javascript and resources to output/embedded folder


I think this comment may not be correct?

mcdurdin · 2018-02-15T00:35:14Z

web/source/kmwRecorder.html

+
+  <!-- Initialization: set paths to keyboards, resources and fonts as required -->
+  <script>
+    var kmw = window.keyman;


The kmw variable is probably overkill now. I'd prefer to see use just using keyman. -- it's short enough and more self-documenting.

Good old holdovers from the old 'uncompiled' sample page. Will clean up.

mcdurdin · 2018-02-15T00:37:55Z

web/source/kmwRecorder.html

+      </td>
+      <td style="width:50%">
+        <p style="font-weight: bold;">Valid OS:</p>
+        <nobr><input type="checkbox" id="platform_any" checked onclick="clearPlatforms();">Any</nobr>


Minor improvment: Replacing the text Any with <label for='platform_any'>Any</label> means the user can click on the text as on the checkbox itself.

mcdurdin · 2018-02-15T00:48:10Z

web/source/kmwosk.ts

+        if(keyName == 'K_LOPT' || keyName == 'K_ROPT') {
+          osk.optionKey(e,keyName,true);
+          return true;
+        } else if(keyName == 'K_BKSP') {


Does this work with keyboards that override the default backspace behaviour (e.g. Khmer Angkor with Coeng markers and subconsonants)?

... thanks for bringing this up. I remember seeing a branch for K_BKSP there before and modified it to match the old behavior, but it's not in master. Not sure how that happened, but it's not a direct correlation to the code that's there now, at the very least. I'll look into it.

For future records: what I saw was on the touch handler, which originally bypassed this method entirely. Which was a bug, but adding the 'K_BKSP' check here was also a bug, since there was code later in the method that could be relied upon.

Oh yeah, it's because it was in touch but the recorder needs clickKey. But the implementation in touch was a bug, as we realized after a little discussion.

mcdurdin · 2018-02-15T00:50:31Z

web/source/kmwutils.ts

-
-    return 999;
+  getIEVersion() {
+    Device._GetIEVersion();


This should return the result from Device._GetIEVersion() shouldn't it?

mcdurdin · 2018-02-15T01:02:26Z

web/unit_tests/cases/basics.js

@@ -31,11 +31,11 @@ Modernizr.on('touchevents', function(result) {
    describe('Basic Toggle UI', function() {

      beforeEach(function(done) {
-        this.timeout(10000);
+        this.timeout(20000);


Is this because the tests are taking longer than 10 seconds to run? It feels a little arbitrary. Is it network or environment dependent?

I think it's network dependent. When the tests are run, they're hosted on the local machine, not on BrowserStack servers; a 'tunnel' of sorts is set up that allows the local machine to act as 'localhost' to their remote machines.

Therefore, the quality of net connection affects the needed timeouts... and our connection here locally isn't always the greatest, so periodically the tests fail when the connection acts up.

Some of the recent script changes in test_utils.js do their best to minimize the impact of this, as even initialization tests are far more able to recognize when they've successfully finished all their work, even when it's dependent on script loading, etc.

jahorton added 13 commits February 9, 2018 14:52

Initial engine unit test infrastructure in place with keyboard load t…

496433a

…est.

Unit test can now simulate a key press. Desktop only; is not yet filt…

fc19f32

…ered from touch tests.

Slight documentation enhancement.

317230b

A click-simulating OSK test has been added to the engine suite.

9b5ef1f

Removed unnecessary timeout delays.

e58312d

Design and partial integration of recording-oriented resources for en…

6d1f718

…gine testing.

Added new InputTestSequence class and integrated a test with it.

12d3a04

Partial touch support - keystrokes on touch-aliased elements are now …

1f35e7a

…testable. Problem: touch events.

Touch emulation now supported. Had to alter true code to run dev test…

b493be2

…s in Chrome emulation.

Added code to capture + reconstruct keyboard stubs from KMW's activeS…

1b2da39

…tub property.

Rudimentary test recording UI is now up for the recorder page.

b3896bd

Recorder page is now Chrome-touch-simulation compatible.

a90cb14

Adjusts the OSK to allow the recorder to detect touch-based deletions.

889d2e2

jahorton added enhancement web/ work-in-progress labels Feb 12, 2018

jahorton added this to the P2S13 milestone Feb 12, 2018

jahorton mentioned this pull request Feb 12, 2018

[Web] Chiral font update #452

Merged

jahorton added 3 commits February 13, 2018 08:43

Merge branch 'web-350-attachment-tests' into web-350-engine-tests

8e1e0bc

Accidental lines left out of merge + necessitated change to the new '…

c0b518e

…engine' test case.

Relocates the recorder into the general /source/ directory + ties its…

6970c86

… build into the test command.

mcdurdin changed the title ~~[WIP] KeymanWeb - Engine testing~~ [web] [WIP] KeymanWeb - Engine testing Feb 13, 2018

jahorton added 8 commits February 13, 2018 11:03

Engine test + recorder build now integrated and working with CI test …

1d88348

…run.

InputTestSequence JSON pretty-print is "go." Fixes recorder post-merge.

607cefd

Moved Device to new file for Recorder access, + objects for auto tests.

99db1cd

Added initial constructor for the KeyboardTest class.

5ec722e

Merge branch 'master' into web-350-engine-tests

18ba780

Recorder infrastructure and UI now only need polish.

7ebb730

The new OO structure is now used by the example engine test case.

428ba42

Minor test runner parameter tweak for readability.

819e37a

jahorton mentioned this pull request Feb 14, 2018

bug(web): HTMLElement.focus() lacks proper touch-oriented handling. #598

Closed

jahorton added 2 commits February 14, 2018 15:28

Tweaks recorder touch support to better load existing JSON.

0f14149

Now takes in per-sequence error messages if specified.

0725a8f

jahorton removed the work-in-progress label Feb 14, 2018

jahorton changed the title ~~[web] [WIP] KeymanWeb - Engine testing~~ [web] KeymanWeb - Engine testing Feb 14, 2018

mcdurdin requested changes Feb 15, 2018

View reviewed changes

jahorton added 3 commits February 15, 2018 08:31

Fixes for PR comments.

ffa76e7

Missed a minor detail and addressed the OSK BKSP key thing.

049f7a7

Merge branch 'master' into web-350-engine-tests

22c1c98

mcdurdin approved these changes Feb 15, 2018

View reviewed changes

jahorton merged commit 460ff66 into master Feb 15, 2018

jahorton deleted the web-350-engine-tests branch February 15, 2018 04:44

jahorton mentioned this pull request May 8, 2018

KMW Mobile-mode: Backspace triggering inconsistencies #716

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[web] KeymanWeb - Engine testing #585

[web] KeymanWeb - Engine testing #585

jahorton commented Feb 12, 2018 •

edited

Loading

mcdurdin left a comment

mcdurdin Feb 15, 2018

mcdurdin Feb 15, 2018

jahorton Feb 15, 2018

mcdurdin Feb 15, 2018

mcdurdin Feb 15, 2018

jahorton Feb 15, 2018

jahorton Feb 15, 2018

jahorton Feb 15, 2018

mcdurdin Feb 15, 2018

mcdurdin Feb 15, 2018

jahorton Feb 15, 2018

[web] KeymanWeb - Engine testing #585

[web] KeymanWeb - Engine testing #585

Conversation

jahorton commented Feb 12, 2018 • edited Loading

Engine Tester

Engine Recorder

mcdurdin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jahorton commented Feb 12, 2018 •

edited

Loading