feat(testing): Provide MockPlatformLocation by default in BrowserTest… #49137
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
35fe111 to
47600ef
Compare
47600ef to
bb4e23d
Compare
Contributor
Author
|
merge assistance: There is a g3 patch that sets |
…ngModule
Tests sometimes do not mock out the `PlatformLocation` and end up
affecting real browser state. This can mean changing the real URL of the
browser during a test, updating the History state object, or any number
of other stateful operations. This can result in a test unintentionally affecting
other tests in the suite because the browser state does not usually get
reset before the next test runs. Providing `MockPlatformLocation` by
default prevents these types of accidental test leakages.
In addition, not providing `MockPlatformLocation` by default led to
developers needing to add `RouterTestingModule` to their test suite to
avoid the problems above. This module has spy `Location` providers which
prevent those issues. This commit now makes `RouterTestingModule`
obsolete. Developers can now just use `RouterModule.forRoot` or
`provideRouter` directly in tests _without_ needing to learn to import
additional test providers or modules.
With this, we should consider deprecating `RouterTestingModule` altogether and
migrating developers to `RouterModule.forRoot` or `provideRouter` instead. There
are some small differences between `SpyLocation` and
`MockPlatformLocation` that might cause tests to fail after the
migration (`MockPlatformLocation` is actually more correct in its
behaviors). If this happens, we can advise developers to also add
`provideLocationMocks()` to their test providers, which would re-provide
the `SpyLocation` like before and should make the tests pass again.
BREAKING CHANGE: `MockPlatformLocation` is now provided by default in tests.
Existing tests may have behaviors which rely on
`BrowserPlatformLocation` instead. For example, direct access to the
`window.history` in either the test or the component rather than going
through the Angular APIs (`Location.getState()`). The quickest fix is to
update the providers in the test suite to override the provider again
`TestBed.configureTestingModule({providers: [{provide: PlatformLocation, useClass: BrowserPlatformLocation}]})`.
The ideal fix would be to update the code to instead be compatible with
`MockPlatformLocation` instead.
bb4e23d to
7468308
Compare
Member
|
This PR was merged into the repository by commit 5dce2a5. |
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ingModule
Tests sometimes do not mock out the
PlatformLocationand end up affecting real browser state. This can mean changing the real URL of the browser during a test, updating the History state object, or any number of other stateful operations. This can result in a test unintentionally affecting other tests in the suite because the browser state does not usually get reset before the next test runs. ProvidingMockPlatformLocationby default prevents these types of accidental test leakages.In addition, not providing
MockPlatformLocationby default led to developers needing to addRouterTestingModuleto their test suite to avoid the problems above. This module has spyLocationproviders which prevent those issues. This commit now makesRouterTestingModuleobsolete. Developers can now just useRouterModule.forRootorprovideRouterdirectly in tests without needing to learn to import additional test providers or modules.With this, we should consider deprecating
RouterTestingModulealtogether and migrating developers toRouterModule.forRootorprovideRouterinstead. There are some small differences betweenSpyLocationandMockPlatformLocationthat might cause tests to fail after the migration (MockPlatformLocationis actually more correct in its behaviors). If this happens, we can advise developers to also addprovideLocationMocks()to their test providers, which would re-provide theSpyLocationlike before and should make the tests pass again.BREAKING CHANGE:
MockPlatformLocationis now provided by default in tests. Existing tests may have behaviors which rely onBrowserPlatformLocationinstead. For example, direct access to thewindow.historyin either the test or the component rather than going through the Angular APIs (Location.getState()). The quickest fix is to update the providers in the test suite to override the provider againTestBed.configureTestingModule({providers: [{provide: PlatformLocation, useClass: BrowserPlatformLocation}]}). The ideal fix would be to update the code to instead be compatible withMockPlatformLocationinstead.