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

feat(web): custom infrastructure for @web/test-runner use 🏃 #11403

Merged
merged 9 commits into from
May 22, 2024

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented May 9, 2024

Try as I might, I couldn't find any reporters for @web/test-runner designed for use with TeamCity. Fortunately, they've got pretty good documentation for their reporter API... so I decided to roll our own. As work went on and I remembered some issues we've had with browser-level automated tests in the past, I also added a couple more features to enhance our build logs and surface data that wasn't easily accessible otherwise.

Obviously, I worked on these after actually migrating many of our automated tests to @web/test-runner. That said... it'd be simpler and more time-efficient to review such PRs with the changes near-finalized out of the gate, so I've hoisted this earlier in the commit chain.

So, the new modules:

  1. test-runner-TC-reporter
    • This formats test results in the format that TeamCity natively parses, allowing us to have nice reports on the tests in build reports. I've also added some checks here to add a small, human-readable summary after the TC-formatting section is complete on which test-files, platforms, etc had failures and why (when available), providing a central point of reference than requiring a search through the build logs to find a lead for test-related build failures.
    • If there were no errors, no "failure report" section appears.
  2. test-runner-rename-browser
    • The BrowserLaunchers support a name field, but don't take custom names when initializing them - at least as far as I can see. Fortunately, they're mutable after construction, so the workaround is pretty simple.
  3. test-runner-stability-reporter.js
    • After some research and code-inspection into our preferred browser-engine launching setup, I found a way to "spy" on calls used to manage browser-engine state in order to surface otherwise-silent errors that can result sometimes when running tests.
    • As I managed to find a scenario in which the test-runner failed against WebKit without providing any relevant information, I felt this important.

Also, rather than require pre-compilation... I just used JSDoc comments to achieve pseudo-TypeScript. That can be changed if desired easily enough. I'm also not sure this is the best place to leave the definitions, so I'd like to address concerns on code location and the JS/TS question at the same time. As it is, this implementation is enough to proceed with further work, so I decided to move on to questions with clearer answers.

See related CI build logs for #11404 (Test - Language Modeling Layer) to inspect and verify the output and usefulness of these reporters.

@keymanapp-test-bot skip

@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S1 milestone May 9, 2024
@@ -94,7 +94,7 @@ function test-headless() {
TEST_OPTS="--reporter mocha-teamcity-reporter"
fi
if [[ -n "$TEST_EXTENSIONS" ]]; then
TEST_OPTS=" --extension $TEST_EXTENSIONS"
TEST_OPTS="$TEST_OPTS --extension $TEST_EXTENSIONS"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I happened to notice that Web's child build scripts weren't outputting their headless-testing results into TC-friendly format even when run by CI. It was a simple fix, and it's kind of related anyway, so I threw the one-liner fix in here.

@jahorton jahorton changed the title feat(web): custom infrastructure for @web/test-runner use feat(web): custom infrastructure for @web/test-runner use 🏃 May 9, 2024
@jahorton
Copy link
Contributor Author

jahorton commented May 9, 2024

From the latest build of #11404:

Here's confirmation that the custom TC-reporter functions as intended.

picture of relevant TC build log section - can expand and collapse like similar Karma logs

picture of TC test-report section - the reported tests are fully recognized by TC

@jahorton jahorton marked this pull request as ready for review May 9, 2024 05:01
@darcywong00 darcywong00 modified the milestones: A18S1, A18S2 May 11, 2024
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, assuming that TODO does not need to be addressed first

common/test/resources/test-runner-TC-reporter.mjs Outdated Show resolved Hide resolved
common/test/resources/test-runner-TC-reporter.mjs Outdated Show resolved Hide resolved
Base automatically changed from chore/web/other-dependency-updates to master May 15, 2024 07:10
@jahorton jahorton merged commit c3f57d5 into master May 22, 2024
15 checks passed
@jahorton jahorton deleted the feat/web/custom-wtr-reporting branch May 22, 2024 14:51
@keyman-server
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants