Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(common/core/web): error from early fat-finger termination due to OS interruptions 🍒 #5491

Merged
merged 6 commits into from
Jul 28, 2021

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Jul 27, 2021

A 🍒-pick of #5479.

User Testing

@keymanapp/testers

It's literally the same set of tests that was used for #5479.

Much of this is taken from standard KMW acceptance testing and adapted toward the aspects of KMW modified by this PR, though we'll only worry about testing against a single platform here.

Platform: Chrome emulation / Android

Ensure that you see the "Console" tab while performing these tests.

  • TEST_CRITICAL: If any errors (red text entries in the "Console" area) or warnings appear during these tests, fail this test entry and report a screenshot of them.
    • Ignore anything seen elsewhere mentioning "Issues" or within a small "Issues" tab - there are a few entries we know about, but they're not affected by this PR. I'm looking for "Console" log messages.

Console tab:

image

It should resemble Windows' CMD prompt and the macOS Terminal.

Utilize the "Test unminified Keymanweb" testing page to ensure the following:

  • TEST_1: KMW properly handles input to the controls.
  • Attempt to add the following keyboards:
    • TEST_2: Using "Add a keyboard by keyboard name", add: sil_ipa. (For our currently-deprecated IPA keyboard.)
      • TEST_3: ŋ should be a subkey of n - ensure it works.
    • TEST_4: Using "Add a keyboard by keyboard name", add: khmer_angkor
      • TEST_5: Type in the following sequence: ស, ុ, ្រ (subkey of រ), ក. You should see ស្រុក.
      • TEST_6: Continuing from the last test, hit backspace in sequence. As this is a reorder (keyboard) rule test, you should see:
        • ស្រុ
        • ស្រ
    • TEST_7: Using "Add a keyboard by language name(s)", add: spanish.

Utilize the "Prediction - robust testing" testing page for the following:

  • Swap to the "English - EuroLatin (SIL)" keyboard.
    • TEST_8: Use long-press . to output ' and then press e. The two characters should not combine.
    • TEST_9: Ensure that long-press p and SHIFT + long-press g displays properly and that the subkeys produce the expected output.
    • TEST_10: Type the following sequence, pressing near the center of the key each time:
      • L (shift layer)
      • k (default layer)
      • v (default layer)
      • Expected result: you should see suggestions for Love and Live, along with one other random suggestion.

@jahorton jahorton added this to the A15S10 milestone Jul 27, 2021
@github-actions github-actions bot added common/ common/core/ common/web/ cherry-pick Change already merged into another (stable) branch fix web/ labels Jul 27, 2021
@jahorton
Copy link
Contributor Author

jahorton commented Jul 27, 2021

Huh... git rebase -i didn't auto-cherry / include the documentation comment that was applied by code suggestion. Odd.

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Any sanity user testing we can do?

@jahorton
Copy link
Contributor Author

LGTM. Any sanity user testing we can do?

"Can do?" We can always run the same tests from the PR against master. There's no reliable way to set up an actual repro to test, though.

@mcdurdin
Copy link
Member

There's no reliable way to set up an actual repro to test, though.

I'm looking more for regression tests -- make sure nothing else gets broken. I'm fairly confident that this is true, but can you add a user test for Makara similar to the one we did for master?

@jahorton
Copy link
Contributor Author

jahorton commented Jul 27, 2021

... but can you add a user test for Makara similar to the one we did for master?

Gave it the ol' CTRL+C -> CTRL+V sequence.

@MakaraSok
Copy link
Collaborator

MakaraSok commented Jul 28, 2021

User Testing

@keymanapp/testers

Platform: Chrome emulation / Android

Build and test on Windows 10 Pro 21H1, Chrome 92.0.4515.107 (x64) - Pixel 2

  • TEST_CRITICAL: FAILED - No errors (red text entries in the "Console" area) or warnings appear during "Test unminified Keymanweb". There is one thing though during "Prediction - robust testing".

This shows up when switching the keyboard to EuroLatin (SIL).
image

Utilize the "Test unminified Keymanweb" testing page to ensure the following:

  • TEST_1: PASSED - the OSK can be used as usual
  • Attempt to add the following keyboards:
    • TEST_2: PASSED - no issue when adding sil_ipa using "Add a keyboard by keyboard name".
      • TEST_3: PASSED - ŋ is one of the subkeys of n - it works as intended.
    • TEST_4: PASSED - no issue when adding khmer_angkor using "Add a keyboard by keyboard name".
      • TEST_5: PASSED - type in ្រ (subkey of ) and the output indeed is ស្រុក.
      • TEST_6: PASSED - Continuing from the last test, hit backspace in sequence. As this is a reorder (keyboard) rule test, each backspace indeed shows:
        • ស្រុ
        • ស្រ
    • TEST_7: PASSED - Using "Add a keyboard by language name(s)", add: spanish and a Spanish keyboard shows up on the keyboard menu off of the globe key

Utilize the "Prediction - robust testing" testing page for the following:

  • Swap to the "English - EuroLatin (SIL)" keyboard.
    • TEST_8: PASSED - Use long-press . to output ' and then press e. The two characters do not combine per expectation.
    • TEST_9: PASSED - long-press p and SHIFT + long-press g displays properly and that the subkeys produce the expected output.
    • TEST_10: PASSED - Type the following sequence, pressing near the center of the key each time:
      • L (shift layer)
      • k (default layer)
      • v (default layer)
      • Expected result: the expected suggestions for Love and Live, along with one other random suggestion are shown.

@jahorton
Copy link
Contributor Author

Eh, that's fine. (The outputContext missing message. Not ideal, but it's definitely not branch-local.) And it's "yellow," not red, so it's not worth blocking on.

@jahorton jahorton merged commit 7f12d5a into stable-14.0 Jul 28, 2021
@jahorton jahorton deleted the fix/common/core/web/cherry/empty-alternates branch July 28, 2021 10:06
@MakaraSok
Copy link
Collaborator

User Testing [retest on Android for good measure]

@keymanapp/testers
@darcywong00 @mcdurdin

Platform: Android 6.0.1 emulated on Nexus 5

Test build #14.0.280-test at 2 Aug 01:52

Utilize the "Test unminified Keymanweb" testing page to ensure the following:

  • TEST_1: PASSED - the OSK can be used as usual
  • Attempt to add the following keyboards:
    • TEST_2: PASSED - no issue when adding sil_ipa using "Add a keyboard by keyboard name".

    successfully installed this from Settings > Install Keyboard or Dictionary > Install from keyman.com > sil_ipa

    • TEST_3: PASSED - ŋ is one of the subkeys of n - it works as intended.
    • TEST_4: PASSED - no issue when adding khmer_angkor using "Add a keyboard by keyboard name".

    successfully installed this from Settings > Install Keyboard or Dictionary > Install from keyman.com > khmer_angkor

    • TEST_5: PASSED - type in ្រ (subkey of ) and the output indeed is ស្រុក.
    • TEST_6: PASSED - Continuing from the last test, hit backspace in sequence. As this is a reorder (keyboard) rule test, each backspace indeed shows:
      • ស្រុ
      • ស្រ
    • TEST_7: PASSED - Using "Add a keyboard by language name(s)", add: spanish and a Spanish keyboard shows up on the keyboard menu off of the globe key

    Tap on the plus sign in Installed Languages>type in the search box Spanish>tap to install EuroLatin (SIL) keyboard for Spanish language>successfully installed.

Utilize the "Prediction - robust testing" testing page for the following:

  • Swap to the "English - EuroLatin (SIL)" keyboard.
    • TEST_8: PASSED - Use long-press . to output ' and then press e. The two characters do not combine per expectation.
    • TEST_9: PASSED - long-press p and SHIFT + long-press g displays properly and that the subkeys produce the expected output.
    • TEST_10: FAILED - Type the following sequence, pressing near the center of the key each time:
      • L (shift layer)
      • k (default layer)
      • v (default layer)
      • Expected result: the expected suggestions for Love and Live, along with one other random suggestion are shown.
      • Actual result: no suggestion given in the banner. See the screenshot below.
      image

@MakaraSok
Copy link
Collaborator

Retested for good measure using the stable build #14.0.280-test at 2 Aug 01:52.

Confirm that all tests have PASSED. Things are working as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick Change already merged into another (stable) branch common/web/ common/ fix has-user-test web/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants