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

Zoneless tests with unset required inputs succeeds unexpectedly #56977

Open
szacskesz opened this issue Jul 13, 2024 · 3 comments · May be fixed by #56993
Open

Zoneless tests with unset required inputs succeeds unexpectedly #56977

szacskesz opened this issue Jul 13, 2024 · 3 comments · May be fixed by #56993
Labels
area: testing Issues related to Angular testing features, such as TestBed core: zoneless Issues related to running Angular without zone.js
Milestone

Comments

@szacskesz
Copy link

Which @angular/* package(s) are the source of the bug?

core, zone.js

Is this a regression?

Yes

Description

I am testing a component with (signal based) required inputs. When no value is provided to the required input, it should throw an NG0950 error. With zone change detection it was the case, but when the provideExperimentalZonelessChangeDetection() is used, the tests succeeds unexpectedly.

After examining the logs I see the NG0950 error logged, but the test still succeeds.
I believe it might be related to #56240

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/stackblitz-starters-pdwsvu?file=src%2Fapp%2Fapp.component.spec.ts

Please provide the exception or error you saw

> ng test

✔ Browser application bundle generation complete.
13 07 2024 15:41:47.999:WARN [karma]: No captured browser, open http:https://localhost:9876/
13 07 2024 15:41:48.010:INFO [karma-server]: Karma v6.4.3 server started at http:https://localhost:9876/
13 07 2024 15:41:48.010:INFO [launcher]: Launching browsers Chrome with concurrency unlimited
13 07 2024 15:41:48.012:INFO [launcher]: Starting browser Chrome
13 07 2024 15:41:48.525:INFO [Chrome 126.0.0.0 (Windows 10)]: Connected on socket PaQI-IqyyYwYYJJcAAAB with id 11717700
ERROR: 'ERROR', Error: NG0950: Input is required but no value is available yet. Find more at https://angular.dev/errors/NG0950
Error: NG0950: Input is required but no value is available yet. Find more at https://angular.dev/errors/NG0950
    at AppComponent.inputValueFn [as a2] (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:98:19)
    at templateFn (ng:https:///AppComponent.js:111:54)
    at executeTemplate (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:11620:9)
    at refreshView (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13238:13)
    at detectChangesInView (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13454:9)
    at detectChangesInViewIfAttached (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13414:5)
    at detectChangesInComponent (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13403:5)
    at detectChangesInChildComponents (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13467:9)
    at refreshView (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13292:13)
    at detectChangesInView (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13454:9)
Chrome 126.0.0.0 (Windows 10): Executed 0 of 1 SUCCESS (0 secs / 0 secs)
ERROR: 'ERROR', Error: NG0950: Input is required but no value is available yet. Find more at https://angular.dev/errors/NG0950
Error: NG0950: Input is required but no value is available yet. Find more at https://angular.dev/errors/NG0950
    at AppComponent.inputValueFn [as a2] (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:98:19)
    at templateFn (ng:https:///AppComponent.js:111:54)
    at executeTemplate (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:11620:9)
    at refreshView (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13238:13)
    at detectChangesInView (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13454:9)
    at detectChangesInViewIfAttached (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13414:5)
    at detectChangesInComponent (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13403:5)
    at detectChangesInChildComponents (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13467:9)
    at refreshView (http:https://localhost:9876/_karma_webpack_/webpack:/node_modules/@angular/core/fesm2022/core.mjs:13292:13)
Chrome 126.0.0.0 (Windows 10): Executed 1 of 1 SUCCESS (0.531 secs / 0.424 secs)
TOTAL: 1 SUCCESS

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 18.1.0
Node: 20.15.0
Package Manager: npm 10.7.0
OS: win32 x64

Angular: 18.1.0
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1801.0
@angular-devkit/build-angular   18.1.0
@angular-devkit/core            18.1.0
@angular-devkit/schematics      18.1.0
@schematics/angular             18.1.0
rxjs                            7.8.1
typescript                      5.5.3
zone.js                         0.14.7

Anything else?

No response

@JeanMeche JeanMeche added area: testing Issues related to Angular testing features, such as TestBed core: zoneless Issues related to running Angular without zone.js labels Jul 13, 2024
@ngbot ngbot bot modified the milestone: needsTriage Jul 13, 2024
@JeanMeche
Copy link
Member

JeanMeche commented Jul 13, 2024

Take away a doubt from me, in the zone app, the error is thrown when you call fixture.detectChanges() right ?

@szacskesz
Copy link
Author

Take away a doubt from me, in the zone app, the error is thrown when you call fixture.detectChanges() right ?

Yes, see the same repro for the zone app:
https://stackblitz.com/edit/stackblitz-starters-tp2ugc?file=src%2Fapp%2Fapp.component.spec.ts

@JeanMeche
Copy link
Member

JeanMeche commented Jul 13, 2024

So yeah basically this is same root cause than #56240.

In the zone app, it's the detectChanges(), a function that is invoked in the test that fires the error.

In the case of the zoneless test, the CD is fired from an ApplicationRef.tick which wraps everything in a try-catch block and logs the errors to the ErrorHandler.

Testing in zoneless apps is a topic in itself that still need some work. You find some details here.

atscott added a commit to atscott/angular that referenced this issue Jul 14, 2024
…oneless tests

The behavior of `ComponentFixture` for zoneless tests was decided somewhat through guesswork, trial, and error. In addition, working on the zoneless fixture revealed oddities in the behavior of the zone-based fixture, or behaviors that we felt were counterintuitive. The most consequential difference is how change detection works: `detectChanges` goes through ApplicationRef.tick in zoneless while it is `changeDetectorRef.detectChanges` in the zone fixture.

We felt that running change detection through `ApplicationRef.tick` was important for several reasons:
* Aligning application behavior more closely with the test behavior (almost all views are attached to application ref in reality)
* Ensuring that afterRender* hooks are executed when calling `fixture.detectChanges`
* Ensuring that the change detection runs again if render hooks update state

This change, however, has some noticeable consequences that will break some tests, mostly around how errors are handled. `ApplicationRef.tick` catches errors that happen during change detection and reports them to the ErrorHandler from DI. The default error handler only logs the error to the console. This will break tests which have `expect(() => fixture.detectChanges()).toThrow()`. In addition, it allows tests to pass when there are real errors encountered during change detection.

This change ensures that errors from `ApplicationRef.tick` are rethrown
and will fail the test. We should also do a follow-up investigation to
determine whether we can/should also do this for the zone-based
`ComponentFixture`.

fixes angular#56977
atscott added a commit to atscott/angular that referenced this issue Jul 15, 2024
…oneless tests

The behavior of `ComponentFixture` for zoneless tests was decided somewhat through guesswork, trial, and error. In addition, working on the zoneless fixture revealed oddities in the behavior of the zone-based fixture, or behaviors that we felt were counterintuitive. The most consequential difference is how change detection works: `detectChanges` goes through ApplicationRef.tick in zoneless while it is `changeDetectorRef.detectChanges` in the zone fixture.

We felt that running change detection through `ApplicationRef.tick` was important for several reasons:
* Aligning application behavior more closely with the test behavior (almost all views are attached to application ref in reality)
* Ensuring that afterRender* hooks are executed when calling `fixture.detectChanges`
* Ensuring that the change detection runs again if render hooks update state

This change, however, has some noticeable consequences that will break some tests, mostly around how errors are handled. `ApplicationRef.tick` catches errors that happen during change detection and reports them to the ErrorHandler from DI. The default error handler only logs the error to the console. This will break tests which have `expect(() => fixture.detectChanges()).toThrow()`. In addition, it allows tests to pass when there are real errors encountered during change detection.

This change ensures that errors from `ApplicationRef.tick` are rethrown
and will fail the test. We should also do a follow-up investigation to
determine whether we can/should also do this for the zone-based
`ComponentFixture`.

fixes angular#56977
atscott added a commit to atscott/angular that referenced this issue Jul 15, 2024
…oneless tests

The behavior of `ComponentFixture` for zoneless tests was decided somewhat through guesswork, trial, and error. In addition, working on the zoneless fixture revealed oddities in the behavior of the zone-based fixture, or behaviors that we felt were counterintuitive. The most consequential difference is how change detection works: `detectChanges` goes through ApplicationRef.tick in zoneless while it is `changeDetectorRef.detectChanges` in the zone fixture.

We felt that running change detection through `ApplicationRef.tick` was important for several reasons:
* Aligning application behavior more closely with the test behavior (almost all views are attached to application ref in reality)
* Ensuring that afterRender* hooks are executed when calling `fixture.detectChanges`
* Ensuring that the change detection runs again if render hooks update state

This change, however, has some noticeable consequences that will break some tests, mostly around how errors are handled. `ApplicationRef.tick` catches errors that happen during change detection and reports them to the ErrorHandler from DI. The default error handler only logs the error to the console. This will break tests which have `expect(() => fixture.detectChanges()).toThrow()`. In addition, it allows tests to pass when there are real errors encountered during change detection.

This change ensures that errors from `ApplicationRef.tick` are rethrown
and will fail the test. We should also do a follow-up investigation to
determine whether we can/should also do this for the zone-based
`ComponentFixture`.

fixes angular#56977
atscott added a commit to atscott/angular that referenced this issue Jul 15, 2024
…oneless tests

The behavior of `ComponentFixture` for zoneless tests was decided somewhat through guesswork, trial, and error. In addition, working on the zoneless fixture revealed oddities in the behavior of the zone-based fixture, or behaviors that we felt were counterintuitive. The most consequential difference is how change detection works: `detectChanges` goes through ApplicationRef.tick in zoneless while it is `changeDetectorRef.detectChanges` in the zone fixture.

We felt that running change detection through `ApplicationRef.tick` was important for several reasons:
* Aligning application behavior more closely with the test behavior (almost all views are attached to application ref in reality)
* Ensuring that afterRender* hooks are executed when calling `fixture.detectChanges`
* Ensuring that the change detection runs again if render hooks update state

This change, however, has some noticeable consequences that will break some tests, mostly around how errors are handled. `ApplicationRef.tick` catches errors that happen during change detection and reports them to the ErrorHandler from DI. The default error handler only logs the error to the console. This will break tests which have `expect(() => fixture.detectChanges()).toThrow()`. In addition, it allows tests to pass when there are real errors encountered during change detection.

This change ensures that errors from `ApplicationRef.tick` are rethrown
and will fail the test. We should also do a follow-up investigation to
determine whether we can/should also do this for the zone-based
`ComponentFixture`.

fixes angular#56977
atscott added a commit to atscott/angular that referenced this issue Jul 15, 2024
…oneless tests

The behavior of `ComponentFixture` for zoneless tests was decided somewhat through guesswork, trial, and error. In addition, working on the zoneless fixture revealed oddities in the behavior of the zone-based fixture, or behaviors that we felt were counterintuitive. The most consequential difference is how change detection works: `detectChanges` goes through ApplicationRef.tick in zoneless while it is `changeDetectorRef.detectChanges` in the zone fixture.

We felt that running change detection through `ApplicationRef.tick` was important for several reasons:
* Aligning application behavior more closely with the test behavior (almost all views are attached to application ref in reality)
* Ensuring that afterRender* hooks are executed when calling `fixture.detectChanges`
* Ensuring that the change detection runs again if render hooks update state

This change, however, has some noticeable consequences that will break some tests, mostly around how errors are handled. `ApplicationRef.tick` catches errors that happen during change detection and reports them to the ErrorHandler from DI. The default error handler only logs the error to the console. This will break tests which have `expect(() => fixture.detectChanges()).toThrow()`. In addition, it allows tests to pass when there are real errors encountered during change detection.

This change ensures that errors from `ApplicationRef.tick` are rethrown
and will fail the test. We should also do a follow-up investigation to
determine whether we can/should also do this for the zone-based
`ComponentFixture`.

fixes angular#56977
atscott added a commit to atscott/angular that referenced this issue Jul 16, 2024
…oneless tests

The behavior of `ComponentFixture` for zoneless tests was decided somewhat through guesswork, trial, and error. In addition, working on the zoneless fixture revealed oddities in the behavior of the zone-based fixture, or behaviors that we felt were counterintuitive. The most consequential difference is how change detection works: `detectChanges` goes through ApplicationRef.tick in zoneless while it is `changeDetectorRef.detectChanges` in the zone fixture.

We felt that running change detection through `ApplicationRef.tick` was important for several reasons:
* Aligning application behavior more closely with the test behavior (almost all views are attached to application ref in reality)
* Ensuring that afterRender* hooks are executed when calling `fixture.detectChanges`
* Ensuring that the change detection runs again if render hooks update state

This change, however, has some noticeable consequences that will break some tests, mostly around how errors are handled. `ApplicationRef.tick` catches errors that happen during change detection and reports them to the ErrorHandler from DI. The default error handler only logs the error to the console. This will break tests which have `expect(() => fixture.detectChanges()).toThrow()`. In addition, it allows tests to pass when there are real errors encountered during change detection.

This change ensures that errors from `ApplicationRef.tick` are rethrown
and will fail the test. We should also do a follow-up investigation to
determine whether we can/should also do this for the zone-based
`ComponentFixture`.

fixes angular#56977
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: testing Issues related to Angular testing features, such as TestBed core: zoneless Issues related to running Angular without zone.js
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants