-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
feat(testing): Provide MockPlatformLocation by default in BrowserTest… #49137
Conversation
35fe111
to
47600ef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
47600ef
to
bb4e23d
Compare
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
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. |
…ingModule
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. ProvidingMockPlatformLocation
by default prevents these types of accidental test leakages.In addition, not providing
MockPlatformLocation
by default led to developers needing to addRouterTestingModule
to their test suite to avoid the problems above. This module has spyLocation
providers which prevent those issues. This commit now makesRouterTestingModule
obsolete. Developers can now just useRouterModule.forRoot
orprovideRouter
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 toRouterModule.forRoot
orprovideRouter
instead. There are some small differences betweenSpyLocation
andMockPlatformLocation
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 addprovideLocationMocks()
to their test providers, which would re-provide theSpyLocation
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 onBrowserPlatformLocation
instead. For example, direct access to thewindow.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 againTestBed.configureTestingModule({providers: [{provide: PlatformLocation, useClass: BrowserPlatformLocation}]})
. The ideal fix would be to update the code to instead be compatible withMockPlatformLocation
instead.