Skip to content

fix(generate): make generated component.spec consistent with AppCompo… #2488

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

Closed

Conversation

lacolaco
Copy link
Contributor

@lacolaco lacolaco commented Oct 3, 2016

…nent spec

ng new generates a little rich spec file for AppComponent

/* tslint:disable:no-unused-variable */

import { TestBed, async } from '@angular/core/testing';
import { AppComponent } from './app.component';

describe('App: test-app', () => {
  beforeEach(() => {
    TestBed.configureTestingModule({
      declarations: [
        AppComponent
      ],
    });
  });

  it('should create the app', async(() => {
    let fixture = TestBed.createComponent(AppComponent);
    let app = fixture.debugElement.componentInstance;
    expect(app).toBeTruthy();
  }));

  it(`should have as title 'app works!'`, async(() => {
    let fixture = TestBed.createComponent(AppComponent);
    let app = fixture.debugElement.componentInstance;
    expect(app.title).toEqual('app works!');
  }));

  it('should render title in a h1 tag', async(() => {
    let fixture = TestBed.createComponent(AppComponent);
    fixture.detectChanges();
    let compiled = fixture.debugElement.nativeElement;
    expect(compiled.querySelector('h1').textContent).toContain('app works!');
  }));
});

But a spec via ng generate component is not so. Imperative component instantiation is not a good pattern, I think.

/* tslint:disable:no-unused-variable */

import { TestBed, async } from '@angular/core/testing';
import { HeroDetailComponent } from './hero-detail.component';

describe('Component: HeroDetail', () => {
  it('should create an instance', () => {
    let component = new HeroDetailComponent();
    expect(component).toBeTruthy();
  });
});

At least, I think it should generate a spec using TestBed.

@lacolaco lacolaco force-pushed the fix-component-spec-consistency branch from 9fc8e90 to c604fe2 Compare October 3, 2016 16:52
@filipesilva
Copy link
Contributor

We actually made it this basic (#1085) in response to user feedback that it was too complicated.

That was a different time though, when TestBed didn't exist. @Brocco do you think we should add it in?

@Brocco
Copy link
Contributor

Brocco commented Oct 10, 2016

I actually prefer this change. Yes, we did simplify, but as @filipesilva pointed out, that was before TestBed was implemented.

This PR adds the suggested test strategy without adding complication or bloat of spec files.

It has my approval, but I won't merge until others have a chance to weigh in. I would like to have teh opinion of @wardbell on this one, because he is passionate about testing and was involved in the spec simplification change.

@wardbell
Copy link
Contributor

wardbell commented Oct 10, 2016

I think there is more of a case for generating a true Component spec, now that the TestBed is so sweet.

However, this isn't that spec. Rather than go on at length about what's amiss with this one, @Brocco, should we use it for inspiration and work up something on the side? You know how to reach me.

@Brocco
Copy link
Contributor

Brocco commented Oct 14, 2016

Closing... handled via PR #2674

@Brocco Brocco closed this Oct 14, 2016
@lacolaco lacolaco deleted the fix-component-spec-consistency branch July 7, 2018 06:56
@angular-automatic-lock-bot
Copy link

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 12, 2019
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.

5 participants