Generation of test: Use Classname.name in describe of spec files instead of 'Classname'

[Rogier-blip]

🚀 feature request

When a spec file is generated by the CLI use Classname.name instead of the 'Classname'.

Relevant Package

@angular/cli

Description

It would be nice if the spec files would be generated in a way that makes them adaptable to change.

If a test is generated by the CLI the initial describe looks like this:

...
describe('AppComponent', () => {
...

When I change the name of the app component from AppComponent to AppComponent1. I have to change the name of the test by hand.

Describe the solution you'd like

The solution that I have in mind is to use the .name property of the typescript class. The describe would have the following structure.

...
describe(AppComponent.name, () => {
...

Now when the AppComponent name is changed through an IDE. The test name is automatically changed.

Cheers 🐵

[alan-agius4]

There is actually a PR for this #18843

[dgp1130]

We had a discussion about this in among the team with a significant amount of bike shedding on this topic.

The main advantage of using AppComponent.name is that using an automated refactor to rename the class will also update the test name. Which is very convenient and I know I personally have forgotten to update describe() names before.

However, there are a few, less obvious downsides to consider:

• Readability: Using a simple string is more clear about the intent of labeling the describe() block, whereas using Class.name takes more mental effort to track. It is also one less layer of indirection, since developers don't have to figure out the meaning of name.
• name property is not well known: Many developers not familiar with many of the intricacies of JavaScript may not know the name property. To them, it looks like its accessing a static name property on the AppComponent class, which doesn't appear to be defined anywhere. It's also technically on the Function prototype, since classes in JavaScript are just functions, which can be unintuitive to someone not familiar with the decades of legacy JavaScript syntax and coming from a more object-oriented language. Such a developer might reasonably think this is some kind of Angular magic included via the @Component() decorator or injected by the Angular compiler. If TS supported a nameof() operator like in C#, this might be a more reasonable change.
• Language service: The TS language service is crashing for me atm, but I believe it does not show name in autocomplete, which can add to confusion about the property.
• Uncommon pattern: It is standard practice to just use string literals in describe() and it() blocks. While that's not universal, there is an advantage to staying with the mainstream and reducing variety in the ecosystem when it is unnecessary.
• Codebase inconsistency: Developers are already using the string literal. If we change the default to the class name reference, then codebases now have two different test formats. If developers want to be consistent, they either need to undo our change via a project/user-level config (and request some flag for us or to change the schematic their using) or update all existing tests to use the new class name reference (manually or via some migration schematic we would need to support).

Ultimately, this distinction is relatively minor and could easily be argued either way. Our consensus is that the marginal refactoring improvement is not worth the additional cost that comes with it. If specific developers/teams really want this behavior, they can make their own schematic that does this or just manually update the describe() after it is generated. Automated refactors won't cover this symbol, however Intellij products can already refactor names in string literals and comments which would cover this use case (I don't believe VSCode has that feature). Even if users forget to refactor the name, it won't actually cause any problems with the test, it just has a bad label which can be trivially fixed when it is eventually noticed by a developer, so the negative impact of this mistake is pretty minor to begin with.

Our decision for now is to continue generating the string literal 'AppComponent' rather than changing it to use AppComponent.name.

[angular-automatic-lock-bot]

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.}