chore(web): updates chai, @types/chai dependencies 🏃 #11318

jahorton · 2024-04-29T08:48:41Z

One major task I identified while working on #11300 (toward resolving #10497) is that we'll need to update our JS testing-assertion library. Chai 4 exists as a CommonJS module, but the modern tools we're looking to migrate toward require ES Module libraries. The Chai team fixed this with version 5, so we may as well update.

This PR's changeset has been directly extracted from an intermediate state of #11300.

This has a few "fun" knock-on effects for our Karma tests; the "framework" loading approach we previously used is not viable with it, as it is not ES-module aware. With a little legwork, we can patch everything up and maintain the tests as they are, just using the new Chai version, before we begin migrating the affected tests to a new testing framework.

@keymanapp-test-bot skip

keymanapp-test-bot · 2024-04-29T08:48:45Z

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

Android
Developer
iOS
- Keyman for iOS (simulator image)
- FirstVoices Keyboards for iOS (simulator image)
- TestFlight internal PR build version - 18.0.30 (0.11318.10924)
Keyboards
- Test Keyboards
Web
- KeymanWeb Test Home
Windows

Updates 'chai' assertion library to latest; old version used CommonJS imports

chore(web): missed leaving these out of the rebase

The upgrade to chai means that karma's 'framework' feature is no longer compatible, meaning a _little_ extra legwork is required to use it for karma browser tests. It's just a little bit, though. Will need to replicate this for the other packages that use karma to test, as the pattern kinda proliferated.

…ests

jahorton · 2024-05-08T01:52:05Z

common/web/types/test/ldml-keyboard/test-ldml-keyboard-xml-reader.ts

@@ -36,7 +36,13 @@ describe('ldml keyboard xml reader tests', function () {
      callback: (data, source, subpath, callbacks) => {
        assert.ok(source?.keyboard3?.keys);
        const k = pluckKeysFromKeybag(source?.keyboard3?.keys.key, ['a', 'b', 'c']);
-        assert.sameDeepOrderedMembers(k, [
+        assert.sameDeepOrderedMembers(k.map((entry) => {
+          // Drop the Symbol members from the returned keys; assertions may expect their presence.


The Chai version update caused the corresponding unit tests to fail; the "actual" objects from pluckKeysFromKeybag have Symbol.___ members, while the "expected" ones specified below do not.

In the previous Chai version, Symbol.___ members were not checked.

jahorton · 2024-05-08T05:13:07Z

Whoa, noticed something really weird about the last Web test run...

https://build.palaso.org/buildConfiguration/Keymanweb_TestPullRequests/460863

09:09:49   07 05 2024 22:09:49.807:DEBUG [launcher.browserstack]: Shutting down BrowserStackLocal
09:09:49   07 05 2024 22:09:49.826:DEBUG [launcher]: FINISHED -> FINISHED
09:09:49   07 05 2024 22:09:49.827:DEBUG [launcher]: FINISHED -> FINISHED
09:09:49   07 05 2024 22:09:49.963:DEBUG [launcher.browserstack]: chrome 100.0 (Windows 10) (worker 122904664) successfully killed.
09:09:49   07 05 2024 22:09:49.963:DEBUG [launcher]: null -> FINISHED
09:09:49   07 05 2024 22:09:49.963:DEBUG [launcher]: FINISHED -> FINISHED
09:09:53   07 05 2024 22:09:53.602:DEBUG [launcher.browserstack]: Stopped BrowserStackLocal
09:09:53   [common/web/gesture-recognizer] ## test:browser failed
09:09:53   [web] ## test failed
09:09:54   [web] build.sh parameters: <coverage>
09:09:55   [web] ## coverage starting...
09:09:55   [web] Creating coverage report...
09:10:02   [web] ## coverage completed successfully
09:10:02   Process exited with code 0

Somehow, the script kept going after the test failure, with the coverage success masking it!? This isn't from changes in this PR, though.

Edit: this script was unadjusted by the recent #11329 and has its set -x disabled. It fully utilizes builder-script functionality, though. That could be related.

jahorton · 2024-05-09T01:09:11Z

common/test/resources/karma-browserstack-config.cjs

  bs_safari_mac_m: {
    browser: 'safari',
-    browser_version: '15.3',
+    browser_version: '17.3',
    os: 'OS X',
-    os_version: 'Monterey'
+    os_version: 'Sonoma'


It seems as though the version of Safari we'd been targeting for testing with BrowserStack didn't like the updated Chai assertion library. There were a number of build failures that mentioned test-loading issues in a manner kind of buried inside the build logs; I'd missed that detail for several builds. Updating the target to a more recent Safari version bypasses the issue.

Our goal with the 🏃 PR series is migrate away from CI testing against BrowserStack anyway, so I consider this a "stopgap" measure we can use to keep the tests passing during the test-migration process.

mcdurdin · 2024-05-13T03:24:14Z

Somehow, the script kept going after the test failure, with the coverage success masking it!?

That's because the build configuration step masks it:

if exist .build-builder (
  node ../resources/gosh/gosh.js ./ci.sh test
) else (
  node ../resources/gosh/gosh.js unit_tests/test.sh -CI
)

find "coverage_action" build.sh > NUL
if not errorlevel 1 (
  node ../resources/gosh/gosh.js ./build.sh coverage
)

Batch files do not exit automatically on failure. You need something like this after each node call:

  if errorlevel 1 exit %errorlevel%

mcdurdin

I'm not a fan of the deep references into node_modules but I guess there isn't a really nice way around that at present?

However, what if we had a stub module called /common/web/test/chai.js which just exported from node_modules:

export { assert, expect } from '../../../../../../../.../../././..,,.,/./.../node_modules/chai/chai.js';

jahorton · 2024-05-15T01:18:03Z

I'm not a fan of the deep references into node_modules but I guess there isn't a really nice way around that at present?

However, what if we had a stub module called /common/web/test/chai.js which just exported from node_modules:
export { assert, expect } from '../../../../../../../.../../././..,,.,/./.../node_modules/chai/chai.js';

Fortunately, once we convert tests to the new format, all those deep-references disappear. We need them for karma (at least, with our existing configurations), but not with @web/test-runner. Actually, I've seen evidence that there may be a plugin out there we could use to fix them even with karma, but I'm not sure it's worth the time investment since karma's on the way out for us. I could chase that if you'd prefer, though.

What you've suggested here is likely a "decent enough" workaround, and would probably be enough if you do want me to go ahead and make the transitional form cleaner.

See #11404 (comment) as a reference for how it gets fixed upon migration.

mcdurdin · 2024-05-15T02:26:56Z

Fortunately, once we convert tests to the new format, all those deep-references disappear.

This is good enough for me. No point in doing extra maintenance on an issue that's about to be resolved anyway.

keyman-server · 2024-05-15T18:57:35Z

Changes in this pull request will be available for download in Keyman version 18.0.36-alpha

keymanapp-test-bot bot added this to the A18S1 milestone Apr 29, 2024

jahorton changed the title ~~chore(web): updates chai, @types/chai~~ chore(web): updates chai, @types/chai dependencies Apr 29, 2024

github-actions bot added web/ developer/ common/ common/models/ developer/compilers/ common/web/ common/models/templates/ common/models/wordbreakers/ developer/ide/ chore labels Apr 29, 2024

jahorton force-pushed the chore/web/chai-update branch from 39041ad to 34cdbb7 Compare April 29, 2024 08:57

jahorton changed the title ~~chore(web): updates chai, @types/chai dependencies~~ chore(web): updates chai, @types/chai dependencies 🏃 Apr 29, 2024

jahorton added 8 commits May 7, 2024 14:35

chore(web): chai dependency updates

5574b94

Updates 'chai' assertion library to latest; old version used CommonJS imports

chore(common): drops package-specific 'chai', '@types/chai' versions

398b9e0

chore(web): missed leaving these out of the rebase

fix(web): gesture-recognizer Karma tests

7a66eb2

fix(web): kbd-proc Karma test fix

2bc2f77

fix(web): dom (single-browser) Karma test adjustments

c744598

fix(web): regression-test test-builder adjustment

ffba4a6

fix(web): fixes Web's Karma test:integrated suite (last one)

3989298

jahorton force-pushed the chore/web/chai-update branch from 34cdbb7 to 3989298 Compare May 7, 2024 07:35

fix(common): Chai 5.0 update strictness broke some common/web/types t…

079e683

…ests

jahorton commented May 8, 2024

View reviewed changes

chore(web): update Safari BrowserStack target as stopgap measure

5c43042

jahorton commented May 9, 2024

View reviewed changes

jahorton marked this pull request as ready for review May 9, 2024 05:01

jahorton requested review from ermshiperete, mcdurdin, darcywong00 and rc-swag as code owners May 9, 2024 05:01

darcywong00 modified the milestones: A18S1, A18S2 May 11, 2024

mcdurdin approved these changes May 13, 2024

View reviewed changes

jahorton merged commit 887a479 into master May 15, 2024
19 checks passed

jahorton deleted the chore/web/chai-update branch May 15, 2024 07:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(web): updates chai, @types/chai dependencies 🏃 #11318

chore(web): updates chai, @types/chai dependencies 🏃 #11318

jahorton commented Apr 29, 2024

keymanapp-test-bot bot commented Apr 29, 2024 •

edited

Loading

jahorton May 8, 2024

jahorton May 8, 2024

jahorton commented May 8, 2024 •

edited

Loading

jahorton May 9, 2024 •

edited

Loading

mcdurdin commented May 13, 2024

mcdurdin left a comment

jahorton commented May 15, 2024 •

edited

Loading

mcdurdin commented May 15, 2024

keyman-server commented May 15, 2024

chore(web): updates chai, @types/chai dependencies 🏃 #11318

chore(web): updates chai, @types/chai dependencies 🏃 #11318

Conversation

jahorton commented Apr 29, 2024

keymanapp-test-bot bot commented Apr 29, 2024 • edited Loading

User Test Results

Test Artifacts

jahorton May 8, 2024

Choose a reason for hiding this comment

jahorton May 8, 2024

Choose a reason for hiding this comment

jahorton commented May 8, 2024 • edited Loading

jahorton May 9, 2024 • edited Loading

Choose a reason for hiding this comment

mcdurdin commented May 13, 2024

mcdurdin left a comment

Choose a reason for hiding this comment

jahorton commented May 15, 2024 • edited Loading

mcdurdin commented May 15, 2024

keyman-server commented May 15, 2024

keymanapp-test-bot bot commented Apr 29, 2024 •

edited

Loading

jahorton commented May 8, 2024 •

edited

Loading

jahorton May 9, 2024 •

edited

Loading

jahorton commented May 15, 2024 •

edited

Loading