Initial Web Test Runner Implementation #25860
Conversation
dgp1130
added
action: review
alan-agius4
added
needs: discussion
|
Sent modernweb-dev/web#2523 to fix the |
dgp1130
force-pushed
the
wtr
branch
2 times, most recently
from
6b8216b to
ae16ded
Compare
|
I'm waiting for this PR to land. I'm wondering, what's blocking this? Is there any estimate on if it'll be part of some Angular 17 version or will it come with 18? I would like to try this out, as an Angular app that I work on has a lot of it's testing blocked by this PR. If there's an external library where things can be iterated or a way to add this specific package from git, please let me know |
I am also eagerly awaiting this feature. What I have done is to compile Angular myself and integrate it temporarily (see developer docs ). In my experience so far, the integration of Web Test Runner works perfectly (big thumbs-up to @dgp1130). However, so far there are no configuration options (see here), which is not a big problem for me at the moment. |
packages/angular_devkit/build_angular/src/builders/web-test-runner/schema.json
Show resolved
Hide resolved
| "polyfills": { | ||
| "description": "Polyfills to be included in the build.", | ||
| "oneOf": [ | ||
| { | ||
| "type": "array", | ||
| "description": "A list of polyfills to include in the build. Can be a full path for a file, relative to the current workspace or module specifier. Example: 'zone.js'.", | ||
| "items": { | ||
| "type": "string", | ||
| "uniqueItems": true | ||
| }, | ||
| "default": [] | ||
| }, | ||
| { | ||
| "type": "string", | ||
| "description": "The full path for the polyfills file, relative to the current workspace or a module specifier. Example: 'zone.js'." | ||
| } | ||
| ] | ||
| }, |
There was a problem hiding this comment.
| "polyfills": { | |
| "description": "Polyfills to be included in the build.", | |
| "oneOf": [ | |
| { | |
| "type": "array", | |
| "description": "A list of polyfills to include in the build. Can be a full path for a file, relative to the current workspace or module specifier. Example: 'zone.js'.", | |
| "items": { | |
| "type": "string", | |
| "uniqueItems": true | |
| }, | |
| "default": [] | |
| }, | |
| { | |
| "type": "string", | |
| "description": "The full path for the polyfills file, relative to the current workspace or a module specifier. Example: 'zone.js'." | |
| } | |
| ] | |
| }, | |
| "polyfills": { | |
| "description": "Polyfills to be included in the build.", | |
| { | |
| "type": "array", | |
| "description": "A list of polyfills to include in the build. Can be a full path for a file, relative to the current workspace or module specifier. Example: 'zone.js'.", | |
| "items": { | |
| "type": "string", | |
| "uniqueItems": true | |
| }, | |
| "default": [] | |
| } | |
| }, |
There was a problem hiding this comment.
Karma supports a string case, so I think we would want WTR to support that as well?
There was a problem hiding this comment.
It comes down if we want to have the same options or not. IE: we want users to switch back and forth from karma.
Since, this is a new builder, we can update and remove unneeded options, and simplify certain options. Similar to how we did for the application builder were one of the changes was to not support the string format.
There was a problem hiding this comment.
We'll need to make those decisions as we start looking at specific options. I'm definitely open to changing and cleaning things up here, however supporting a schematic migration from Karma to WTR is in scope here, so whatever changes we do make will need to be automatable. I don't think we need to worry too much about users switching back to Karma, but we do want to make sure the migration to WTR is as smooth as we can make it.
There was a problem hiding this comment.
This migration should remain unaffected as long as the implementations are incorporated later.
Since this is an experimental phase and we won't be generating projects using this version, I'm comfortable leaving it unchanged for now. However, users opting for this builder might face some challenges as options will undergo changes in the future.
Therefore, I see two potential paths forward:
- Remove all presently unused options and introduce them gradually as their implementations are integrated, aligning them with the application builder options.
- Maintain the current setup and revise or remove the options before finalizing this builder. This, naturally, implies more disruptions for users already using WTR.
There was a problem hiding this comment.
I'm not too concerned about disrupting experimental users, that is the contract we have for experimental. I was mainly following the browser-esbuild precedent of having compatible options with warnings for unsupported ones. I think this will evolve over time during the experimental period, and as long as we nail it down prior to stable / dev preview, I'm not too concerned about the churn.
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Outdated
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Outdated
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Outdated
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/builders/web-test-runner/builder-status-warnings.ts
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Outdated
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Show resolved
Hide resolved
dgp1130
force-pushed
the
wtr
branch
5 times, most recently
from
d211a38 to
8a545b7
Compare
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Outdated
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Outdated
Show resolved
Hide resolved
packages/angular_devkit/build_angular/src/builders/web-test-runner/index.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Can you also please update
angular-cli/packages/angular/cli/lib/config/workspace-schema.json
Lines 346 to 410 in 6999c2e
dgp1130
force-pushed
the
wtr
branch
2 times, most recently
from
0431028 to
98c58ed
Compare
Ah totally forgot about that again. Updated. |
|
|
||
| /** Listens for a single emission of an event and returns the value emitted. */ | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| function once<Map extends Record<string, any>, EventKey extends string & keyof Map>( |
There was a problem hiding this comment.
Is this because wtr doesn't expose an actual EventEmitter?
Node has nice helpers for the promise use case: https://nodejs.org/api/events.html#eventsonceemitter-name-options
There was a problem hiding this comment.
Interesting, I wasn't aware of a built-in API for this.
Web Test Runner seems to be using that under the hood, but doesn't expose it. Their EventEmitter does not extend or implement Node's EventEmitter: https://github.com/modernweb-dev/web/blob/245123e1b92fe0d468fc7ae1e25663f52e775bea/packages/test-runner-core/src/utils/EventEmitter.ts
Those utilities seem nice, and maybe it's compatible enough for them to work, but I'm hesitant to rely on that if we can avoid it.
This is somewhat temporary anyways, once we get to a real watch mode, once will go away entirely.
packages/angular_devkit/build_angular/src/builders/web-test-runner/options.ts
Outdated
Show resolved
Hide resolved
alan-agius4
removed
the
target: major
alan-agius4
added
action: merge
…reused in multiple builders
… bare strings as module specifiers Previously an entry point `foo` would be incorrectly treated as a relative path, which meant it was not possible to use an NPM package as an entry point. This now requires `./foo` for relative paths while `foo` is treated as a module specifier referring to an NPM package called `foo`.
…ion of `@web/test-runner` builder
This is a new `@angular-devkit/build-angular:web-test-runner` builder which invokes Web Test Runner to execute unit tests in a real browser.
The implementation calls `application` builder under the hood with some option overrides build the application to a temporary directory and then runs Web Test Runner on the output. This set up is still minimal, but sufficient to run and pass tests in the generated `ng new` application.
The `schema.json` file is directly copied from the `karma` builder, since this is intended to serve as a migration target for users coming from Karma. Most of the options don't actually work yet, which is logged when they are used.
The most interesting part of this change is configuring Jasmine to execute in Web Test Runner. This is done through the `testRunnerHtml` option which allows us to control the HTML page tests are executed on. We use `test_page.html` which very carefully controls the loading process. I opted to make a single `<script type="module">` which dynamic imports all the relevant pieces so the ordering can be directly controlled more easily. This is better than trying to manage multiple `<script>` tags and pass data between them. Ideally everything would be bundled into a single entry point, however this is not feasible due to the way that ordering requirements do not align with typical `import` structure. Jasmine must come before polyfills which must come before the runner which invokes user code. In an ideal world, this ordering relationship would be represented in `import` statements, but this is not practically feasible because Angular CLI doesn't own all the files (`./polyfills.js` is user-defined) and Jasmine's loading must be split into two places so Zone.js can properly patch it.
`jasmine_runner.js` serves the purpose of executing Jasmine tests and reporting their results to Web Test Runner. I tried to write `jasmine_runner.js` in TypeScript and compile it with a `ts_library`. Unfortunately I don't think this is feasible because it needs to import `@web/test-runner-core` at runtime. This dependency has some code generated at runtime in Web Test Runner, meaning we cannot bundle this dependency and must mark it as external and dynamic `import()` the package at runtime. This works fine in native ESM, but compiling with TypeScript outputs CommonJS code by default (and I don't believe our `@build_bazel_rules_nodejs` setup can easily change that), so any `import('@web/test-runner-core')` becomes `require('@web/test-runner-core')` which fails because that package is ESM-only. The `loadEsmModule` trick does work here either because Web Test Runner is applying Node module resolution at serve time, meaning it looks for `import('@web/test-runner-core')` and rewrites it to something like `import('/node_modules/@web/test-runner-core')`. In short, there is no easy syntax which circumvents the TypeScript compiler while also being statically analyzable to Web Test Runner.
This helps notify users when they are attempting to use an option from Karma which hasn't been implemented yet.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This adds a new
@angular-devkit/build-angular:web-test-runnerbuilder which invokes Web Test Runner to execute tests. The initial implementation is still experimental and lacks a lot of features, but it is sufficient to run tests generated byng new.There's no docs or setup instructions yet, however the main path to trying this out is to replace
@angular-devkit/build-angular:karmawith@angular-devkit/build-angular:web-test-runnerinangular.json. Since Web Test Runner is intended to be a drop-in replacement for Karma, options should mostly align. Currently only a couple options are actually supported right now, but more will be added over time.The commit messages go more in detail on the implementation. In particular, Web Test Runner integration is done via a custom HTML page which explicitly loads the user's JavaScript in a very particular order. This allows our custom Jasmine integration, sets up Zone.js to patch those symbols correctly, configures
TestBed, and then runs the user's tests.Currently some unrelated unit tests appear to be failing due to Web Test Runner pulling in
@types/mochawhich is conflicting with@types/jasmine. I'm looking into the proper fix for that.