search

keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
391 stars 109 forks source link

feat(web): [meta] Better unit-test flakiness reporting #6721

Open jahorton opened 2 years ago

jahorton commented 2 years ago

Every now and again, our BrowserStack-based unit tests for Web and its predictive text engine fall over... simply due to an inability for one or more test configuration to connect properly so that the test may run. When this occurs... there's been nothing in the build logs that explicitly states this. The closest "canary" has been to note the number of "ignored" tests reported; if the number is lower than normal, that usually means at least one testing configuration failed to connect.

With #6707's changes, there's now at least a buried message in the build logs about the failed "capture." This is technically an improvement. That said... it'd be really nice if that warning message were noticed as a warning message by TeamCity, our CI system.

Well... I went and did a little digging; while it would likely be a little 'hacky', we could develop a light 'reporter' module that could listen for capture-failure log messages and surface them in a format that TeamCity would notice. How?

  1. Internally, karma uses log4js for its logging. The BrowserStack plugins for it tie into that for their logging too.

    • Most notably, this setup includes named log caching.

    • If two or more plugins request a logging object using the same ID, they actually use the same logging instance.

      • That ID? launcher.browserstack

    • From there, we can use a 'prototype extension' technique to intercept calls to the relevant logging functions; this would allow us to intercept "failure to capture" messages.

      [JavaScript Unit Tests] [33m05 06 2022 19:53:51.217:WARN [launcher.browserstack]: [39mchrome 70.0 (Windows 10) has not captured in 600000 ms, killing.

    • From there, we can construct a TC-friendly message to indicate warnings and failures that will actually surface.

  2. To tap into this, we could create our own custom "reporter" module and set it up to fetch that logging object.
    • A decent reference for this: https://github.com/karma-runner/karma-teamcity-reporter/blob/master/index.js
      • Note the .$inject field later on - definitions here allow selection of the constructor's parameters!
      • Alternative, karma uses a library called "DI" that is capable of a sort of pseudo-reflection in JavaScript. Using the (signfiicant!) assumption that parameter names directly match the names of objects within the system, it can dynamically build a "default" for .$inject if it is not specified. But again, note that very significant assumption.
        • Relevant name: logger. This exact string must be used either as parameter name or as an entry in the .$inject field in order to connect to the logger module.
        • BrowserStack actually uses this assumption for its karma plugins. (Took me a fair bit of investigation and debugger-tracing to figure it out.)

From there, we could either surface a plain but TC-friendly WARN/ERROR message, or we could go the extra mile to make it look like a failed test. Either one would make things much more explicitly "visible at a glance" when viewing related build logs.

jahorton commented 2 years ago

While the https://github.com/ds300/patch-package package does exist, which would allow us to bypass creation of our own reporter, our current viewpoint is that we'd rather avoid the "patching" approach.