Replace Karma with Web Test Runner #316

shamilovtim · 2023-11-27T17:23:05Z

Reason for PR

Per #232 I spent a few hours on each different testing solution.

Jest. I found Jest to be very powerful. The amount of plugins, preprocessors, configurations, environments it runs in, etc. was impressive. However, it required significant changes to the test suite. There's a codemod for this but it's still a lot of diffs to review for maintainers.

Bare playwright. I tried to run the entire mocha test suite inside of a webpage, with browser orchestration controlled by playwright rather than Karma. This is fine it's also imperative rather than declarative. It requires me to use esbuild to bundle the tests and the browser bundle, and then shove them into a webpage with the Playwright API. I had to manually bootstrap a playwright config, setup all of the hooks, etc. There's an entire npm package that I could create to do this but I'm not sure it's worth it given that's what Web Test Runner does -- injecting a bundle in a site and controlling the orchestration with a declarative config and abstracting away the complexity.

Web Test Runner: Which brings me to this PR. It's similar to the "bare playwright" idea but it's declarative rather than imperative. Most things are managed by the web-test-runner.config.cjs which takes care of all of the implementation details of running something inside of playwright. The one imperative part is the fact that that I do need to bundle with esbuild first. This has implications for --watch mode, however we could simply introduce our own --watch if needed by wrapping the bundling in a watcher. It seems there's almost no getting around jankiness like this until we migrate to the much more powerful Jest.

I propose a two-phase strategy for replacing Karma:

Step 1: Employ Web Test Runner as an immediate Karma substitute, as exemplified by this PR.
Step 2: Migrate to the more powerful Jest, a more challenging but beneficial transition.

Implementation

Here I'm running Karma and WTR to compare them to each other. I kept the Karma implementation in since gutting out Karma is unnecessary to demo this POC and isolates the PR nicely while providing a comparison.

Bootstrap an esbuild config
Bootstrap a web test runner config
Fix chai-as-promised, be sure to chai.use() it everywhere it's necessary.
Bump relevant packages
Add a CI run to compare performance

codesandbox · 2023-11-27T17:23:08Z

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

github-actions · 2023-11-27T17:25:41Z

TBDocs Report

✅ No errors or warnings

@web5/api

Project entry file: packages/api/src/index.ts

TBDocs Report Updated at 2023-12-01T17:18:55Z 9902026

codecov · 2023-11-27T17:28:03Z

Codecov Report

Merging #316 (9902026) into main (a23b49a) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #316   +/-   ##
=======================================
  Coverage   91.77%   91.77%           
=======================================
  Files          73       73           
  Lines       15765    15765           
  Branches     1448     1448           
=======================================
  Hits        14469    14469           
  Misses       1270     1270           
  Partials       26       26

Components	Coverage Δ
api	`96.69% <ø> (ø)`
common	`95.00% <ø> (ø)`
credentials	`94.32% <ø> (ø)`
crypto	`100.00% <ø> (ø)`
dids	`88.75% <ø> (ø)`
agent	`88.08% <ø> (ø)`
identity-agent	`56.81% <ø> (ø)`
proxy-agent	`58.43% <ø> (ø)`
user-agent	`55.22% <ø> (ø)`

shamilovtim · 2023-11-27T18:22:27Z

I had to shutoff playwright webkit because it's hanging. It seems to be responsible for the majority of the issues on CI, along with the macos runner. I am going to submit that we should simply use the ubuntu runner which seems less noisy and still gets us chrome and firefox with less flakes and problems.

packages/agent/karma.conf.cjs

shamilovtim · 2023-11-28T06:52:36Z

Looks like Karma and WTR performance on a single CI vCore is at parity. WTR is 30% faster locally on my machine.

This is a sidegrade. But given no test changes and no deprecation it seems like a potentially fruitful POC. Certainly far less work than all of the Jasmine influenced test runners (Jest / Vitest / bun) which will require huge PR reviews.

shamilovtim · 2023-11-28T23:10:09Z

If anyone enjoys test --watch mode while they're testing or working I can probably easily get it added to both dwn and web5 repos on node and browser. The building blocks are all there, it's just unimplemented.

shamilovtim

Added back Webkit. Speed is pretty much identical between the two solutions except on local machines where WTR outperforms.

Biggest advantages of WTR though:

sourcemaps working well
not deprecated / actively maintained
playwright actually controlling the browsers not karma

Disadvantages of WTR:

manually bundle step rather than automated preprocessor
watch mode will need to be manually implemented by me

shamilovtim · 2023-11-29T23:08:35Z

.github/workflows/tests-ci.yml

@@ -151,7 +147,7 @@ jobs:

      - name: TBDocs Reporter
        id: tbdocs-reporter-protocol
-        uses: TBD54566975/tbdocs@main
+        uses: TBD54566975/tbdocs@b38d87d0c0dca088d3a82e99beea921586c67f67


fixes TBDocs failure that just popped up in the last few hours

frankhinek

SDK implementers need to regularly override the dwn-server instance used in tests which is why the TEST_DWN_URL env var exists. It should be possible to do this for both Node and Browser tests and should not be hard coded to http://localhost:3000.

Can you revert the removal in this PR?

shamilovtim · 2023-11-30T19:06:54Z

Added back an env var override for DWN URL

shamilovtim · 2023-11-30T23:03:31Z

.github/workflows/tests-ci.yml

@@ -124,9 +124,7 @@ jobs:
        run: until curl -sf http://localhost:3000/health; do echo -n .; sleep .1; done

      - name: Run tests for all packages
-        run: npm run test:browser --ws -- --color
-        env:
-          TEST_DWN_URL: http://localhost:3000


this is the same URL as default. no value here.

frankhinek · 2023-12-01T17:07:10Z

packages/agent/package.json

@@ -96,7 +95,6 @@
    "eslint-plugin-mocha": "10.1.0",
    "mocha": "10.2.0",
    "node-stdlib-browser": "1.2.0",
-    "playwright": "1.40.1",


@shamilovtim pretty sure we need to keep the @playwright/test dev dependencies.

every time we've tried to remove them in the past and only have @playwright/test as a dev dep, someone reports issues running tests.

* poc: web test runner * add ci test * webkit is hanging * naming * shutoff webkit * unbump playwright * remove manual timeouts * use concurrency 1 * wrap up poc * run CI for poc back to back with previous impl * tie up loose ends * let wtr run to compare * add back webkit. apples to apples with karma. * remove karma * remove karma * fix tbdocs (i assume) * update ci config * better naming of scripts * fix node * revert test config changes * fix env var override * cleanup ci config * cleanup explicit deps * add back playwright

shamilovtim added 2 commits November 27, 2023 12:01

poc: web test runner

64843fc

add ci test

7d96ca7

shamilovtim requested review from frankhinek, csuwildcat and mistermoe as code owners November 27, 2023 17:23

shamilovtim added 2 commits November 27, 2023 13:06

webkit is hanging

ca19c8b

naming

c43850a

shamilovtim added 6 commits November 27, 2023 13:37

shutoff webkit

c09d2f7

unbump playwright

a04ac05

remove manual timeouts

9e93208

use concurrency 1

56cf9d0

wrap up poc

b075b1c

run CI for poc back to back with previous impl

41ae6ec

shamilovtim commented Nov 28, 2023

View reviewed changes

packages/agent/karma.conf.cjs Show resolved Hide resolved

shamilovtim added 2 commits November 28, 2023 19:40

tie up loose ends

f48c325

let wtr run to compare

aacf06c

frankhinek mentioned this pull request Nov 29, 2023

Test for did:dht signed VCs + release #322

Merged

shamilovtim added 3 commits November 29, 2023 10:32

Merge branch 'main' into tshamilov/web-test-runner

7566abc

add back webkit. apples to apples with karma.

210b693

Merge branch 'main' into tshamilov/web-test-runner

138c155

shamilovtim commented Nov 29, 2023

View reviewed changes

shamilovtim added 2 commits November 29, 2023 16:51

remove karma

cfdd6a3

remove karma

bb9d6ce

shamilovtim changed the title ~~POC: Replace Karma with Web Test Runner~~ Replace Karma with Web Test Runner Nov 29, 2023

shamilovtim added 4 commits November 29, 2023 17:32

fix tbdocs (i assume)

1a374f3

update ci config

de7ee5d

better naming of scripts

092f1b9

fix node

d99e6de

shamilovtim commented Nov 29, 2023

View reviewed changes

frankhinek suggested changes Nov 30, 2023

View reviewed changes

revert test config changes

defbbf7

shamilovtim added the testing related to new or existing tests label Nov 30, 2023

shamilovtim self-assigned this Nov 30, 2023

fix env var override

1e861f6

shamilovtim requested a review from frankhinek November 30, 2023 18:59

shamilovtim added 2 commits November 30, 2023 13:59

cleanup ci config

1ee3e5a

cleanup explicit deps

df95935

Merge branch 'main' into tshamilov/web-test-runner

489f2a5

shamilovtim commented Nov 30, 2023

View reviewed changes

frankhinek reviewed Dec 1, 2023

View reviewed changes

add back playwright

9902026

frankhinek approved these changes Dec 1, 2023

View reviewed changes

csuwildcat approved these changes Dec 1, 2023

View reviewed changes

shamilovtim merged commit ae975ea into main Dec 1, 2023
30 checks passed

shamilovtim deleted the tshamilov/web-test-runner branch December 1, 2023 17:43

frankhinek mentioned this pull request Dec 12, 2023

Replace Karma with another runner for browser tests #232

Closed

frankhinek mentioned this pull request Jan 10, 2024

Replace Karma with Web Test Runner decentralized-identity/dwn-sdk-js#657

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Karma with Web Test Runner #316

Replace Karma with Web Test Runner #316

shamilovtim commented Nov 27, 2023 •

edited

Loading

codesandbox bot commented Nov 27, 2023

github-actions bot commented Nov 27, 2023 •

edited

Loading

codecov bot commented Nov 27, 2023 •

edited

Loading

shamilovtim commented Nov 27, 2023

shamilovtim commented Nov 28, 2023 •

edited

Loading

shamilovtim commented Nov 28, 2023

shamilovtim left a comment •

edited

Loading

shamilovtim Nov 29, 2023

frankhinek left a comment •

edited

Loading

shamilovtim commented Nov 30, 2023

shamilovtim Nov 30, 2023

frankhinek Dec 1, 2023

shamilovtim Dec 1, 2023

Replace Karma with Web Test Runner #316

Replace Karma with Web Test Runner #316

Conversation

shamilovtim commented Nov 27, 2023 • edited Loading

Reason for PR

Implementation

codesandbox bot commented Nov 27, 2023

Review or Edit in CodeSandbox

github-actions bot commented Nov 27, 2023 • edited Loading

@web5/api

codecov bot commented Nov 27, 2023 • edited Loading

Codecov Report

shamilovtim commented Nov 27, 2023

shamilovtim commented Nov 28, 2023 • edited Loading

shamilovtim commented Nov 28, 2023

shamilovtim left a comment • edited Loading

Choose a reason for hiding this comment

shamilovtim Nov 29, 2023

Choose a reason for hiding this comment

frankhinek left a comment • edited Loading

Choose a reason for hiding this comment

shamilovtim commented Nov 30, 2023

shamilovtim Nov 30, 2023

Choose a reason for hiding this comment

frankhinek Dec 1, 2023

Choose a reason for hiding this comment

shamilovtim Dec 1, 2023

Choose a reason for hiding this comment

shamilovtim commented Nov 27, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023 •

edited

Loading

codecov bot commented Nov 27, 2023 •

edited

Loading

shamilovtim commented Nov 28, 2023 •

edited

Loading

shamilovtim left a comment •

edited

Loading

frankhinek left a comment •

edited

Loading