feat(router): Add test helper for trigger navigations in tests #48552

atscott · 2022-12-20T17:47:51Z

In order to test components and services which depend on router navigations, such as ActivatedRoute instances, tests currently need to provide a fair bit of boilerplate or they can set up a stub for ActivatedRoute and list it in the providers to override it in TestBed. This approach of stubbing the ActivatedRoute creates a situation that can easily cause the test to break. The stub often only mocks out the dependencies that the component/service currently needs. This dependencies might change over time and break the test in an unexpected way. In addition, it is difficult to get the structure of ActivatedRoute exactly correct.

This change will allow unit tests to quickly set up routes, trigger real navigations in the Router, and get instances of component's to test along with real instances of ActivatedRoute. This all comes without needing to know that the component depends on ActivatedRoute at all. This becomes more important when considering that a component may be refactored in the future to use @Input rather than access data on the ActivatedRoute instance (see #18967). Tests which mock out ActivatedRoute would all break, but those which use navigateForTest would continue to work without needing any updates.

resolves #15779
resolves #48608

AndrewKushnir

Looks great, just added a few comments.

packages/router/testing/src/navigate_for_test.ts

atscott · 2023-01-05T21:38:26Z

presubmit with local patch to BUILD

caretaker note: when syncing to g3, it will require a manual update to the BUILD file as shown in the linked CL.

goldens/public-api/router/testing/index.md

jessicajaniuk

reviewed-for: fw-testing, public-api

devversion

LGTM for bazel

packages/router/testing/test/BUILD.bazel

…sable location There may be use-cases in the future where `afterNextNavigation` is used in other locations. For example, a test helper function which navigates should likely wait for all redirects to complete rather than simply waiting for the promise returned from `navigate` to resolve.

In order to test components and services which depend on router navigations, such as `ActivatedRoute` instances, tests currently need to provide a fair bit of boilerplate _or_ they can set up a stub for `ActivatedRoute` and list it in the `providers` to override it in `TestBed`. This approach of stubbing the `ActivatedRoute` creates a situation that can easily cause the test to break. The stub often only mocks out the dependencies that the component/service _currently_ needs. This dependencies might change over time and break the test in an unexpected way. In addition, it is difficult to get the structure of `ActivatedRoute` exactly correct. This change will allow unit tests to quickly set up routes, trigger real navigations in the Router, and get instances of component's to test along with real instances of `ActivatedRoute`. This all comes without needing to know that the component depends on `ActivatedRoute` at all. This becomes more important when considering that a component may be refactored in the future to use `@Input` rather than access data on the `ActivatedRoute` instance (see angular#18967). Tests which mock out `ActivatedRoute` would all break, but those which use `navigateForTest` would continue to work without needing any updates. resolves angular#15779 resolves angular#48608

jessicajaniuk

This is a great API change. I really like the RouterTestingHarness approach.

reviewed-for: fw-testing, public-api

AndrewKushnir

@atscott looks great 👍 Just one minor proposal from the public API perspective.

packages/router/testing/src/router_testing_harness.ts

AndrewKushnir

Reviewed-for: public-api

jessicajaniuk · 2023-01-25T19:31:40Z

This PR was merged into the repository by commit dedac8d.

In order to test components and services which depend on router navigations, such as `ActivatedRoute` instances, tests currently need to provide a fair bit of boilerplate _or_ they can set up a stub for `ActivatedRoute` and list it in the `providers` to override it in `TestBed`. This approach of stubbing the `ActivatedRoute` creates a situation that can easily cause the test to break. The stub often only mocks out the dependencies that the component/service _currently_ needs. This dependencies might change over time and break the test in an unexpected way. In addition, it is difficult to get the structure of `ActivatedRoute` exactly correct. This change will allow unit tests to quickly set up routes, trigger real navigations in the Router, and get instances of component's to test along with real instances of `ActivatedRoute`. This all comes without needing to know that the component depends on `ActivatedRoute` at all. This becomes more important when considering that a component may be refactored in the future to use `@Input` rather than access data on the `ActivatedRoute` instance (see #18967). Tests which mock out `ActivatedRoute` would all break, but those which use `navigateForTest` would continue to work without needing any updates. resolves #15779 resolves #48608 PR Close #48552

…ar#48552) In order to test components and services which depend on router navigations, such as `ActivatedRoute` instances, tests currently need to provide a fair bit of boilerplate _or_ they can set up a stub for `ActivatedRoute` and list it in the `providers` to override it in `TestBed`. This approach of stubbing the `ActivatedRoute` creates a situation that can easily cause the test to break. The stub often only mocks out the dependencies that the component/service _currently_ needs. This dependencies might change over time and break the test in an unexpected way. In addition, it is difficult to get the structure of `ActivatedRoute` exactly correct. This change will allow unit tests to quickly set up routes, trigger real navigations in the Router, and get instances of component's to test along with real instances of `ActivatedRoute`. This all comes without needing to know that the component depends on `ActivatedRoute` at all. This becomes more important when considering that a component may be refactored in the future to use `@Input` rather than access data on the `ActivatedRoute` instance (see angular#18967). Tests which mock out `ActivatedRoute` would all break, but those which use `navigateForTest` would continue to work without needing any updates. resolves angular#15779 resolves angular#48608 PR Close angular#48552

…sable location (angular#48552) There may be use-cases in the future where `afterNextNavigation` is used in other locations. For example, a test helper function which navigates should likely wait for all redirects to complete rather than simply waiting for the promise returned from `navigate` to resolve. PR Close angular#48552

…ar#48552) In order to test components and services which depend on router navigations, such as `ActivatedRoute` instances, tests currently need to provide a fair bit of boilerplate _or_ they can set up a stub for `ActivatedRoute` and list it in the `providers` to override it in `TestBed`. This approach of stubbing the `ActivatedRoute` creates a situation that can easily cause the test to break. The stub often only mocks out the dependencies that the component/service _currently_ needs. This dependencies might change over time and break the test in an unexpected way. In addition, it is difficult to get the structure of `ActivatedRoute` exactly correct. This change will allow unit tests to quickly set up routes, trigger real navigations in the Router, and get instances of component's to test along with real instances of `ActivatedRoute`. This all comes without needing to know that the component depends on `ActivatedRoute` at all. This becomes more important when considering that a component may be refactored in the future to use `@Input` rather than access data on the `ActivatedRoute` instance (see angular#18967). Tests which mock out `ActivatedRoute` would all break, but those which use `navigateForTest` would continue to work without needing any updates. resolves angular#15779 resolves angular#48608 PR Close angular#48552

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fanimations/15.1.5/15.2.0) | | [@angular/common](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fcommon/15.1.5/15.2.0) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/15.1.5/15.2.0) | | [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/15.1.5/15.2.0) | | [@angular/core](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fcore/15.1.5/15.2.0) | | [@angular/forms](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fforms/15.1.5/15.2.0) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/15.1.5/15.2.0) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | minor | [`15.1.5` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/15.1.5/15.2.0) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v15.2.0`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1520-2023-02-22) [Compare Source](angular/angular@15.1.5...15.2.0) #### Deprecations ##### - Class and `InjectionToken` guards and resolvers are deprecated. Instead, write guards as plain JavaScript functions and inject dependencies with `inject` from `@angular/core`. ##### | Commit | Type | Description | | -- | -- | -- | | [926c35f4ac](angular/angular@926c35f) | docs | Deprecate class and InjectionToken and resolvers ([#47924](angular/angular#47924)) | ##### common | Commit | Type | Description | | -- | -- | -- | | [54b24eb40f](angular/angular@54b24eb) | feat | Add loaderParams attribute to NgOptimizedImage ([#48907](angular/angular#48907)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [0cf11167f1](angular/angular@0cf1116) | fix | incorrectly detecting forward refs when symbol already exists in file ([#48988](angular/angular#48988)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [a154db8a81](angular/angular@a154db8) | feat | add ng generate schematic to convert declarations to standalone ([#48790](angular/angular#48790)) | | [345e737daa](angular/angular@345e737) | feat | add ng generate schematic to convert to standalone bootstrapping APIs ([#48848](angular/angular#48848)) | | [e7318fc758](angular/angular@e7318fc) | feat | add ng generate schematic to remove unnecessary modules ([#48832](angular/angular#48832)) | ##### language-service | Commit | Type | Description | | -- | -- | -- | | [4ae384fd61](angular/angular@4ae384f) | feat | Allow auto-imports of a pipe via quick fix when its selector is used, both directly and via reexports. ([#48354](angular/angular#48354)) | | [141333411e](angular/angular@1413334) | feat | Introduce a new NgModuleIndex, and use it to suggest re-exports. ([#48354](angular/angular#48354)) | | [d0145033bd](angular/angular@d014503) | fix | generate forwardRef for same file imports ([#48898](angular/angular#48898)) | ##### migrations | Commit | Type | Description | | -- | -- | -- | | [2796230e95](angular/angular@2796230) | fix | add `enum` in `mode` option in `standalone` schema ([#48851](angular/angular#48851)) | | [816e76a578](angular/angular@816e76a) | fix | automatically prune root module after bootstrap step ([#49030](angular/angular#49030)) | | [bdbf21d04b](angular/angular@bdbf21d) | fix | avoid generating imports with forward slashes ([#48993](angular/angular#48993)) | | [32cf4e5cb9](angular/angular@32cf4e5) | fix | avoid internal modules when generating imports ([#48958](angular/angular#48958)) | | [521ccfbe6c](angular/angular@521ccfb) | fix | avoid interrupting the migration if language service lookup fails ([#49010](angular/angular#49010)) | | [a40cd47aa7](angular/angular@a40cd47) | fix | avoid modifying testing modules without declarations ([#48921](angular/angular#48921)) | | [1afa6ed322](angular/angular@1afa6ed) | fix | don't add ModuleWithProviders to standalone test components ([#48987](angular/angular#48987)) | | [c98c6a8452](angular/angular@c98c6a8) | fix | don't copy animations modules into the imports of test components ([#49147](angular/angular#49147)) | | [8389557848](angular/angular@8389557) | fix | don't copy unmigrated declarations into imports array ([#48882](angular/angular#48882)) | | [f82bdc4b01](angular/angular@f82bdc4) | fix | don't delete classes that may provide dependencies transitively ([#48866](angular/angular#48866)) | | [759db12e0b](angular/angular@759db12) | fix | duplicated comments on migrated classes ([#48966](angular/angular#48966)) | | [ba38178d19](angular/angular@ba38178) | fix | generate forwardRef for same file imports ([#48898](angular/angular#48898)) | | [03fcb36cfd](angular/angular@03fcb36) | fix | migrate HttpClientModule to provideHttpClient() ([#48949](angular/angular#48949)) | | [2de6dae16d](angular/angular@2de6dae) | fix | migrate RouterModule.forRoot with a config object to use features ([#48935](angular/angular#48935)) | | [770191cf1f](angular/angular@770191c) | fix | migrate tests when switching to standalone bootstrap API ([#48987](angular/angular#48987)) | | [c7926b5773](angular/angular@c7926b5) | fix | move standalone migrations into imports ([#48987](angular/angular#48987)) | | [65c74ed93e](angular/angular@65c74ed) | fix | normalize paths to posix ([#48850](angular/angular#48850)) | | [6377487b1a](angular/angular@6377487) | fix | only exclude bootstrapped declarations from initial standalone migration ([#48987](angular/angular#48987)) | | [e9e4449a43](angular/angular@e9e4449) | fix | preserve tsconfig in standalone migration ([#48987](angular/angular#48987)) | | [ffad1b49d9](angular/angular@ffad1b4) | fix | reduce number of files that need to be checked ([#48987](angular/angular#48987)) | | [ba7a757cc5](angular/angular@ba7a757) | fix | return correct alias when conflicting import exists ([#49139](angular/angular#49139)) | | [49a7c9f94a](angular/angular@49a7c9f) | fix | standalone migration incorrectly throwing path error for multi app projects ([#48958](angular/angular#48958)) | | [584976e6c8](angular/angular@584976e) | fix | support --defaults in standalone migration ([#48921](angular/angular#48921)) | | [03f47ac901](angular/angular@03f47ac) | fix | use consistent quotes in generated imports ([#48876](angular/angular#48876)) | | [ebae506d89](angular/angular@ebae506) | fix | use import remapper in root component ([#49046](angular/angular#49046)) | | [40c976c909](angular/angular@40c976c) | fix | use NgForOf instead of NgFor ([#49022](angular/angular#49022)) | | [4ac25b2aff](angular/angular@4ac25b2) | perf | avoid re-traversing nodes when resolving bootstrap call dependencies ([#49010](angular/angular#49010)) | | [26cb7ab2e6](angular/angular@26cb7ab) | perf | speed up language service lookups ([#49010](angular/angular#49010)) | ##### platform-browser | Commit | Type | Description | | -- | -- | -- | | [bf4ad38117](angular/angular@bf4ad38) | fix | remove styles from DOM of destroyed components ([#48298](angular/angular#48298)) | ##### platform-server | Commit | Type | Description | | -- | -- | -- | | [25e220a23a](angular/angular@25e220a) | fix | avoid duplicate TransferState info after renderApplication call ([#49094](angular/angular#49094)) | ##### router | Commit | Type | Description | | -- | -- | -- | | [31b94c762f](angular/angular@31b94c7) | feat | Add a withNavigationErrorHandler feature to provideRouter ([#48551](angular/angular#48551)) | | [dedac8d3f7](angular/angular@dedac8d) | feat | Add test helper for trigger navigations in tests ([#48552](angular/angular#48552)) | #### Special Thanks Alan Agius, Alex Castle, Alex Rickabaugh, Andrew Kushnir, Andrew Scott, Dylan Hunn, Ikko Eltociear Ashimine, Ilyass, Jessica Janiuk, Joey Perrott, John Manners, Kalbarczyk, Kristiyan Kostadinov, Matthieu Riegler, Paul Gschwendtner, Pawel Kozlowski, Virginia Dooley, Walid Bouguima, cexbrayat and mgechev  </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1793 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

angular-automatic-lock-bot · 2023-02-25T16:06:28Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

atscott added area: router target: minor This PR is targeted for the next minor release labels Dec 20, 2022

ngbot bot added this to the Backlog milestone Dec 20, 2022

pullapprove bot requested review from AndrewKushnir, jessicajaniuk and pkozlowski-opensource December 20, 2022 17:48

angular-robot bot added the detected: feature PR contains a feature commit label Dec 20, 2022

atscott force-pushed the navigateForTest branch from 9ccc67c to e9670c2 Compare December 20, 2022 17:51

atscott mentioned this pull request Dec 20, 2022

docs(router): Update component testing docs to use RouterTestingHarness #48553

Closed

atscott force-pushed the navigateForTest branch from e9670c2 to a4fb27d Compare December 20, 2022 18:26

AndrewKushnir reviewed Dec 20, 2022

View reviewed changes

packages/router/testing/src/navigate_for_test.ts Outdated Show resolved Hide resolved

packages/router/testing/src/navigate_for_test.ts Outdated Show resolved Hide resolved

packages/router/testing/src/navigate_for_test.ts Outdated Show resolved Hide resolved

pullapprove bot requested a review from dylhunn December 20, 2022 19:26

atscott force-pushed the navigateForTest branch 3 times, most recently from e6fd295 to ae9c064 Compare December 21, 2022 00:10

atscott mentioned this pull request Dec 29, 2022

Change detection should run automatically after router navigation in unit tests #48608

Closed

atscott force-pushed the navigateForTest branch from ae9c064 to a8b8cc4 Compare December 29, 2022 18:39

atscott requested a review from alxhub January 5, 2023 20:45

atscott force-pushed the navigateForTest branch 2 times, most recently from 593f42c to 5ae3ab8 Compare January 5, 2023 21:07

jessicajaniuk reviewed Jan 5, 2023

View reviewed changes

goldens/public-api/router/testing/index.md Outdated Show resolved Hide resolved

atscott force-pushed the navigateForTest branch from f21c8fa to 23f49ce Compare January 5, 2023 22:47

jessicajaniuk approved these changes Jan 5, 2023

View reviewed changes

atscott force-pushed the navigateForTest branch 2 times, most recently from de361e3 to 9dacde0 Compare January 6, 2023 22:59

atscott added the aio: preview label Jan 6, 2023

atscott force-pushed the navigateForTest branch 2 times, most recently from 9ff952f to e5dedc6 Compare January 6, 2023 23:14

pullapprove bot requested a review from devversion January 6, 2023 23:14

devversion approved these changes Jan 9, 2023

View reviewed changes

packages/router/testing/test/BUILD.bazel Outdated Show resolved Hide resolved

atscott force-pushed the navigateForTest branch 2 times, most recently from c46fa0d to e6809e3 Compare January 11, 2023 17:47

atscott added 2 commits January 11, 2023 09:48

atscott force-pushed the navigateForTest branch from e6809e3 to 667af8e Compare January 11, 2023 17:48

atscott requested review from AndrewKushnir and jessicajaniuk January 11, 2023 21:36

fixup! feat(router): Add test helper for trigger navigations in tests

bca35e7

atscott force-pushed the navigateForTest branch from 667af8e to bca35e7 Compare January 11, 2023 21:37

jessicajaniuk approved these changes Jan 11, 2023

View reviewed changes

AndrewKushnir reviewed Jan 12, 2023

View reviewed changes

packages/router/testing/src/router_testing_harness.ts Outdated Show resolved Hide resolved

fixup! feat(router): Add test helper for trigger navigations in tests

d3d6d24

atscott force-pushed the navigateForTest branch from 64014f9 to d3d6d24 Compare January 13, 2023 22:13

AndrewKushnir approved these changes Jan 14, 2023

View reviewed changes

dylhunn approved these changes Jan 21, 2023

View reviewed changes

atscott added action: merge The PR is ready for merge by the caretaker labels Jan 25, 2023

atscott removed request for pkozlowski-opensource and alxhub January 25, 2023 19:17

jessicajaniuk closed this in 930020c Jan 25, 2023

angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(router): Add test helper for trigger navigations in tests #48552

feat(router): Add test helper for trigger navigations in tests #48552

atscott commented Dec 20, 2022 •

edited

AndrewKushnir left a comment

atscott commented Jan 5, 2023

jessicajaniuk left a comment

devversion left a comment

jessicajaniuk left a comment

AndrewKushnir left a comment

AndrewKushnir left a comment

jessicajaniuk commented Jan 25, 2023

angular-automatic-lock-bot bot commented Feb 25, 2023

feat(router): Add test helper for trigger navigations in tests #48552

feat(router): Add test helper for trigger navigations in tests #48552

Conversation

atscott commented Dec 20, 2022 • edited

AndrewKushnir left a comment

Choose a reason for hiding this comment

atscott commented Jan 5, 2023

jessicajaniuk left a comment

Choose a reason for hiding this comment

devversion left a comment

Choose a reason for hiding this comment

jessicajaniuk left a comment

Choose a reason for hiding this comment

AndrewKushnir left a comment

Choose a reason for hiding this comment

AndrewKushnir left a comment

Choose a reason for hiding this comment

jessicajaniuk commented Jan 25, 2023

angular-automatic-lock-bot bot commented Feb 25, 2023

atscott commented Dec 20, 2022 •

edited