Skip to content

chore(test): make test descriptions consistent - #3112

Merged
hansl merged 1 commit into
angular:masterfrom
intellix:test-naming-consistency
Nov 11, 2016
Merged

chore(test): make test descriptions consistent#3112
hansl merged 1 commit into
angular:masterfrom
intellix:test-naming-consistency

Conversation

@intellix

@intellix intellix commented Nov 11, 2016

Copy link
Copy Markdown
Contributor

Components are called: <%= classifiedModuleName %>Directive and it was bothering me that the rest were like: Service: <%= classifiedModuleName %>.

Another change I'd like to make but would like approval first, is to wrap the first generated test in another describe like so:

describe('.constructor()', () => {
  it('should create', () => {
    expect(component).toBeTruthy();
  });
});

I think it makes tests a little more organised and easier to read. I got the idea from reading the Http specs within angular/angular: https://github.com/angular/angular/blob/master/modules/%40angular/http/test/http_spec.ts#L108

@hansl

hansl commented Nov 11, 2016

Copy link
Copy Markdown
Contributor

@johnpapa @Brocco @filipesilva What do you think? We use that standard in our own tests (describe('ClassName', () => ...)), so I don't see why our generated projects should be different. I'll defer to the styleguide for this one though.

@johnpapa

Copy link
Copy Markdown
Contributor

@hansl you mean wrapping the it in a describe ? yes, i agree 100% helps for output organization and the eventual and imminent beforeEach-ness

@hansl

hansl commented Nov 11, 2016

Copy link
Copy Markdown
Contributor

@johnpapa No, look at the changes.

Basically the tests generated by the CLI right now look like this:

describe('Component: Blah', () => {
  // ...
});

With this PR it would look like this:

describe('BlahComponent', () => {
  // ...
});

(the name of the class itself is BlahComponent, but the user likely ran ng generate component blah)

@johnpapa

Copy link
Copy Markdown
Contributor

sorry, a few different things in the comments.

yes, i agree. FooComponent is a much better naming convention

@hansl

hansl commented Nov 11, 2016

Copy link
Copy Markdown
Contributor

LGTM.

@hansl
hansl merged commit ed0fd3e into angular:master Nov 11, 2016
MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017
@angular-automatic-lock-bot

Copy link
Copy Markdown

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.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 11, 2019
@intellix
intellix deleted the test-naming-consistency branch September 11, 2019 22:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants