ionic-team/starters

Angular starters are inconsistent with tests and `beforeEach()` strategy

kensodemann opened this issue · 0 comments

Starter Type: angular
Starter Template: all of them

Description:

The strategy for the beforeEach() in the test templates is inconsistent. While some deviation between variants is required (such as providing routes in some cases but not all), the page and component tests should use a consistent overall strategy and should be clean (no unused ES6 imports, etc.)

blank

  beforeEach(async () => {
    fixture = TestBed.createComponent(HomePage);
    component = fixture.componentInstance;
    fixture.detectChanges();
  });
  1. Minimalist, which is nice.
  2. The async is not needed as-is.
  3. Adding the TestBed.configureTestingModule().compileComponents() back into the template, while not required, would provide a good place to hang overrideProvider() calls for mocking, in which case the async is needed.

sidemenu

  beforeEach(async () => {
    await TestBed.configureTestingModule({
      imports: [AppComponent, IonicModule],
      providers: [provideRouter([])],
    }).compileComponents();
  });
  1. App component test imports IonicModule and it does not need to as that is done in the component itself, so there is no need to also import it in the testing module.
  2. Also, there are unused ES6 imports.
  3. Folders has the same unneeded import of IonicModule but is otherwise OK.

Note: the app component test actually has similar issues in all starters, just pointing it out here...

tabs

  beforeEach(async () => {
    await TestBed.configureTestingModule({
      imports: [Tab1Page, IonicModule, ExploreContainerComponent],
    }).compileComponents();

    fixture = TestBed.createComponent(Tab1Page);
    component = fixture.componentInstance;
    fixture.detectChanges();
  });
  1. The TestBed.configureTestingModule() is not technically needed, but as noted with the blank starter it is very handy.
  2. The IonicModule and ExploreContainerComponent are not needed to be imported in the testing module as the page component handles this

list

  beforeEach(async () => {
    await TestBed.configureTestingModule({
      imports: [HomePage, IonicModule, MessageComponent],
      providers: [provideRouter([])],
    }).compileComponents();

    fixture = TestBed.createComponent(HomePage);
    component = fixture.componentInstance;
    fixture.detectChanges();
  });
  1. consistent with the tabs starter, but not the blank starter
  2. same comment as previous about extra imports that are actually handled already by the component under test

ionic generate page XXXX

Generates a page with a test that is consistent with the HomePage test in the blank starter, but with syntax error (already reported elsewhere)

my-first-app

This one is just plain out of date on many levels, so I am ignoring it for now.

My ionic info:

➜  ionic-training-decks git:(feature/v7-angular-final) ✗ ionic info
[WARN] Error loading @ionic/angular package.json: Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './package' is
       not defined by "exports" in
       /Users/ken/Projects/Training/ionic-training-decks/node_modules/@ionic/angular/package.json

Ionic:

   Ionic CLI                     : 7.0.0 (/Users/ken/.volta/tools/image/packages/@ionic/cli/lib/node_modules/@ionic/cli)
   Ionic Framework               : not installed
   @angular-devkit/build-angular : 15.2.4
   @angular-devkit/schematics    : 15.2.4
   @angular/cli                  : 15.2.4
   @ionic/angular-toolkit        : 8.0.0

Capacitor:

   Capacitor CLI      : 4.7.1
   @capacitor/android : 4.7.1
   @capacitor/core    : 4.7.1
   @capacitor/ios     : 4.7.1

Utility:

   cordova-res : 0.15.4
   native-run  : 1.7.2

System:

   NodeJS : v18.14.2 (/Users/ken/.volta/tools/image/node/18.14.2/bin/node)
   npm    : 9.6.2
   OS     : macOS Unknown

Other Information:
I am very willing to do a PR if we come up with an agreement what the general pattern actually should be.