Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Make unit tests zoneless#4899

Open
JanneSaukkio wants to merge 23 commits into
ng-bootstrap:masterng-bootstrap/ng-bootstrap:masterfrom
JanneSaukkio:spec-zonelessJanneSaukkio/ng-bootstrap:spec-zonelessCopy head branch name to clipboard
Open

Make unit tests zoneless#4899
JanneSaukkio wants to merge 23 commits into
ng-bootstrap:masterng-bootstrap/ng-bootstrap:masterfrom
JanneSaukkio:spec-zonelessJanneSaukkio/ng-bootstrap:spec-zonelessCopy head branch name to clipboard

Conversation

@JanneSaukkio

@JanneSaukkio JanneSaukkio commented Feb 7, 2026

Copy link
Copy Markdown

I might as well update all the remaining unit tests to work as zoneless. Changes to tests of each component are divided into its own commits and there are some extra cleanup.

Ref #4879

@JanneSaukkio

Copy link
Copy Markdown
Author

@quentinderoubaix rebased for Angular 22. All tests pass on Chromium.

@quentinderoubaix

Copy link
Copy Markdown
Contributor

@JanneSaukkio thanks for the contribution!

would you mind rebasing one more time (included playwright 1.61.0)?

I will review afterwards, and my expectations for a merge are:

  • ci should run successfully, so yarn ci
  • if some units are flaky on firefox or webkit, it's fine to skip them for those browsers but never Chromium
  • no call to fixture.detectChanges or fixture.autoDetectChanges left

@JanneSaukkio

Copy link
Copy Markdown
Author

would you mind rebasing one more time (included playwright 1.61.0)?

Will do.

I will review afterwards, and my expectations for a merge are:
...
* no call to fixture.detectChanges or fixture.autoDetectChanges left

Making this change will take a while. There are over 1000 fixture.detectChanges calls in tests and simply replacing them all it with fixture.whenStable will break more than 150 test cases. I will try my best in next few days.

@quentinderoubaix

Copy link
Copy Markdown
Contributor

would you mind rebasing one more time (included playwright 1.61.0)?

Will do.

I will review afterwards, and my expectations for a merge are:
...

  • no call to fixture.detectChanges or fixture.autoDetectChanges left

Making this change will take a while. There are over 1000 fixture.detectChanges calls in tests and simply replacing them all it with fixture.whenStable will break more than 150 test cases. I will try my best in next few days.

We can proceed component per component, smaller scope and easier review too.

@JanneSaukkio

Copy link
Copy Markdown
Author

Earlier changes:

  • Replaced Promise.resolve() calls with fixture.whenStable()
  • Converted test component properties to signals and unused properties removed
  • Replaced some componentInstance.property = <value> calls with bindings inputs (i.e. modal-backdrop.spec.ts:17)
  • Removed NgZone.isInAngularZone() assertions
  • Removed provideZoneChangeDetection() calls

New changes made:

  • Replaced fixture.detectChanges() calls with fixture.whenStable(). This means that most of the test cases are now async functions
  • Removed configuration for running tests with Zone.js

No changes have been made to the library code itself.

I haven't changed any test assertions apart from converting them to async/await in one case (see datepicker.spec.ts:178). The few test cases that have been modified or conditionally skipped are in separate commits.

Each component/module receives beforementioned changes in its own commit. I can break this PR to multiple ones if it's easier for you to review.

There are around 30 failing Firefox and Webkit test cases when running yarn ngb:test. Failing test cases are from spec files that were converted to zoneless earlier: rating, dropdown, nav, pagination and collapse. I haven't made any meaningful changes to these files and cannot debug failures because test pass in watch mode.

There are some "OnPush TestComponents" left to cleanup. It's best to clean them in separate PR after other changes have been merged.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.