feat(migrations): Adds migration for deprecated router testing module #64217
Conversation
Introduces a schematic to replace deprecated router testing imports
| import { Location } from '@angular/common'; | ||
|
|
||
| describe('test', () => { | ||
| let mockLocation : Location; |
There was a problem hiding this comment.
This is a bit heavy-handed. Most things about Location would be the same without provideLocationMocks. The two main things that differ would be: Location.path() when the path is empty returns '/' with the location mocks but returns '' without them. Second, SpyLocation is really the one that's not available unless provideLocationMocks is used, and specifically just SpyLocation.urlChanges.
I would propose adjusting the migration instead:
- Only provide location mocks if
SpyLocation.urlChangesis used - [Optional] attempt to update any
Location.path()expectations to expect empty string instead of'/'. This is somewhat difficult since it differs between test frameworks. I could also see providing the mocks if there is a string literal anywhere in the file that is'/'along with some call toLocation.path() - Update the documentation for the migration to note these differences in functionality with and without
provideLocationMocks.
There was a problem hiding this comment.
Updated with the recommendations, the changes now comply with point 1.
Regarding point 2, it sounds interesting, but it would introduce additional complexity that may vary depending on the test framework. It could potentially be considered under a feature flag, though I’m not entirely sure if it might cause more issues.
Regarding point 3, I’ve mentioned it in the updated documentation, but I’m not sure if it should be more detailed. I could also expand the router testing section to cover the behavior of SpyLocation and provideLocationMocks, since I haven’t found documentation about them, and it might be worth adding this information.
Introduces a schematic to replace deprecated router testing imports
|
This PR was merged into the repository. The changes were merged into the following branches:
|
|
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. |
Adds a schematics migration to convert
RouterTestingModuleusages in test suites toRouterModule, and to addprovideLocationMocks()whenLocationorLocationStrategyis imported and no custom providers exist.Usage
Example
Before:
After:
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #54853
What is the new behavior?
Does this PR introduce a breaking change?
Other information
By default, the regex for the files should be .spec.ts.
Should we make this configurable by the user , since each user might want to set it up differently?
Additional question, would it be good to also consider migration for the HttpClientTestingModule, as a mode of this test migration or should we generate another independent one?