You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "kimyenac (via GitHub)" <gi...@apache.org> on 2026/08/24 08:45:20 UTC
[PR] [ZEPPELIN-6637] Compile Angular decorators in the shell unit test setup [zeppelin]
kimyenac opened a new pull request, #5435:
URL: https://github.com/apache/zeppelin/pull/5435
### What is this PR for?
ZEPPELIN-6567 gave the Angular shell a vitest setup, but only for specs that build their subject by hand. The spec it added is named "mounts React remotes outside the Angular zone without TestBed" and constructs the directive with `new ReactMountDirective(...)`. A spec that goes through TestBed does not run yet, for three separate reasons.
Vitest 4 transforms specs with oxc, and that transform does not apply the decorator options the application build uses. They live in `tsconfig.base.json`, which `src/tsconfig.json` extends while excluding `**/*.spec.ts`. A decorated spec therefore fails to parse with `SyntaxError: Invalid or unexpected token`. Declaring `oxc.decorator` on the vitest config settles it independently of that lookup.
`emitDecoratorMetadata` then compiles to a `__metadata` helper that is a silent no-op unless `Reflect.metadata` exists, so Angular JIT sees no `design:paramtypes` and constructor injection fails with `NG0202`. `src/polyfills.ts` pairs `zone.js` with `core-js/es7/reflect` for the application, and the setup already mirrors the first half, so mirroring the second needs no new dependency.
Finally the setup never calls `TestBed.initTestEnvironment()`, and with vitest globals disabled Angular cannot install its per-test reset hook, so the second spec in a file hits `Cannot configure the test module when the test module has already been instantiated`.
One note on the issue description. It attributes all of this to oxc not reading `experimentalDecorators` and `emitDecoratorMetadata`, and suggests importing `@angular/compiler`. The parse failure is real, but the missing polyfill and the missing TestBed initialization are separate causes, and `@angular/compiler` turns out not to be needed: `@angular/core/testing` already imports it, so the JIT facade is loaded either way.
The TestBed spec for `ReactMountDirective` is the first consumer, and sits alongside the hand-driven one rather than replacing it. It renders the directive from a host template, so the decorator metadata, the `@Input` bindings and the constructor injection all have to resolve for it to run at all. It also provides zone change detection the way `main.ts` does, without which the zone assertions would pass vacuously against TestBed's zoneless default.
None of the three files ships in the application bundle, so there is no runtime change.
### What type of PR is it?
Improvement
### Todos
None
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-6637
### How should this be tested?
`npm run test:shell` from `zeppelin-web-angular`, which is green at 5 specs across 2 files. Each piece of the setup was checked by removing it:
* the `oxc` block: the TestBed spec fails to parse, `SyntaxError: Invalid or unexpected token`
* `core-js/es7/reflect`: 3 failures, `NG0202: This constructor is not compatible with Angular Dependency Injection`
* `initTestEnvironment`: `Need to call TestBed.initTestEnvironment() first`
* `afterEach(resetTestingModule)`: the second spec onward fails with `test module has already been instantiated`
* and to check the new spec is not asserting vacuously, changing the directive's `runOutsideAngular` to `run` makes its zone assertion fail
Note that unit tests do not gate CI yet (ZEPPELIN-6566), so this suite has to be run locally for now.
### Screenshots (if appropriate)
No, this changes test tooling only.
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6637] Compile Angular decorators in the shell unit test setup [zeppelin]
Posted by "voidmatcha (via GitHub)" <gi...@apache.org>.
voidmatcha commented on PR #5435:
URL: https://github.com/apache/zeppelin/pull/5435#issuecomment-5396937210
LGTM 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6637] Compile Angular decorators in the shell unit test setup [zeppelin]
Posted by "voidmatcha (via GitHub)" <gi...@apache.org>.
voidmatcha commented on PR #5435:
URL: https://github.com/apache/zeppelin/pull/5435#issuecomment-5426516734
Merged into master (419d7029).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6637] Compile Angular decorators in the shell unit test setup [zeppelin]
Posted by "voidmatcha (via GitHub)" <gi...@apache.org>.
voidmatcha merged PR #5435:
URL: https://github.com/apache/zeppelin/pull/5435
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org