Skip to content

Commit

Permalink
Migrate Angular router link tests to use RouterTestingHarness (#2430)
Browse files Browse the repository at this point in the history
# Pull Request

## 🤨 Rationale

We have several tests that verify Angular router link behavior by
clicking links and inspecting router state. These tests exhibit a couple
problems:
1. currently they report warnings in the output log like: `[web-server]:
404: /_karma_webpack_/page1?param1=true`
2. In #2387 when we bring in a newer version of Chromium, these cause
other tests to fail to execute and report timeouts

The root cause of these issues is that the tests are actually trying to
navigate the page when the link is clicked.

## 👩‍💻 Implementation

### "with-href" `nimbleRouterLink` tests

In researching best practices for writing tests like this I learned that
the `RouterTestingModule` we had been using has been deprecated and
replaced with a more powerful `RouterTestingHarness`. [This blog and
video](https://www.rainerhahnekamp.com/en/how-do-i-test-using-the-routertestingharness/)
does a good job of explaining it, much better than [the angular
docs](https://angular.dev/api/router/testing/RouterTestingHarness). The
basic idea is that the harness sets up a parent component and router
which host your component under test. When something tries to navigate
the harness captures information about the navigation but doesn't
actually navigate the page.

The fixes in this PR are to use `RouterTestingHarness` instead of
`RouterTestingModule`. This has these side effects:
1. The routes are configured with `provideRouter` instead of
`withRoutes`
2. The navigated route started to be relative to the current route, so
starting from `/start` and clicking a link to `page` resulted in a URL
of `/start/page`. The simplest fix I found for this was to change the
starting page to `/`.
3. Some setup code could be deleted and made sync/async instead of
fakeAsync.

In addition even if the router doesn't navigate the page we are still
invoking click handlers on anchors which try to navigate the page. To
address that I'm calling `preventDefault()` from those handlers.

### error `routerLink` tests

These tests included a `RouterTestingModule` but didn't actually need it
or `RouterTestingHarness` so I deleted those imports.

### package.json

Added a couple dev scripts that I found useful while running tests
locally. We didn't have an obvious way to debug angular tests or to run
just the tests for one project and now we do.

## 🧪 Testing

1. Verified the 404 warnings are no longer printed to the console
5. Verified the tests don't navigate the page in the newer version of
Chromium
6. Verified tests still fail with various changes like changing the URL
or not clicking the link

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <[email protected]>
  • Loading branch information
jattasNI and rajsite authored Oct 9, 2024
1 parent f293446 commit 943ce38
Show file tree
Hide file tree
Showing 15 changed files with 137 additions and 150 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Angular router link test improvements",
"packageName": "@ni/nimble-angular",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Component, ElementRef, Sanitizer, SecurityContext, ViewChild } from '@angular/core';
import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
import { Router } from '@angular/router';
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { provideRouter, Router } from '@angular/router';
import { CommonModule, Location } from '@angular/common';
import { RouterTestingModule } from '@angular/router/testing';
import { RouterTestingHarness } from '@angular/router/testing';
import { parameterizeSpec } from '@ni/jasmine-parameterized';
import { processUpdates } from '../../../testing/async-helpers';
import { NimbleAnchorButtonModule } from '../nimble-anchor-button.module';
Expand All @@ -14,7 +15,6 @@ describe('Nimble anchor button RouterLinkWithHrefDirective', () => {
<nimble-anchor-button #anchor nimbleRouterLink="page1" [queryParams]="{param1: true}" [state]="{stateProperty: 123}">
Anchor text
</nimble-anchor-button>
<router-outlet></router-outlet>
`
})
class TestHostComponent {
Expand All @@ -25,49 +25,47 @@ describe('Nimble anchor button RouterLinkWithHrefDirective', () => {
class BlankComponent { }

let anchorButton: AnchorButton;
let fixture: ComponentFixture<TestHostComponent>;
let testHostComponent: TestHostComponent;
let router: Router;
let location: Location;
let innerAnchor: HTMLAnchorElement;
let routerNavigateByUrlSpy: jasmine.Spy;
let anchorClickHandlerSpy: jasmine.Spy;
let sanitizer: jasmine.SpyObj<Sanitizer>;
let harness: RouterTestingHarness;

beforeEach(() => {
beforeEach(async () => {
sanitizer = jasmine.createSpyObj<Sanitizer>('Sanitizer', ['sanitize']);
sanitizer.sanitize.and.callFake((_, value: string) => value);

TestBed.configureTestingModule({
declarations: [TestHostComponent, BlankComponent],
imports: [NimbleAnchorButtonModule,
imports: [
NimbleAnchorButtonModule,
CommonModule,
RouterTestingModule.withRoutes([
{ path: '', redirectTo: '/start', pathMatch: 'full' },
{ path: 'page1', component: BlankComponent },
{ path: 'start', component: TestHostComponent }
], { useHash: true })
],
providers: [
{ provide: Sanitizer, useValue: sanitizer }
{ provide: Sanitizer, useValue: sanitizer },
provideRouter([
{ path: 'page1', component: BlankComponent },
{ path: '', component: TestHostComponent }
]),
]
});
harness = await RouterTestingHarness.create('');
});

beforeEach(fakeAsync(() => {
beforeEach(() => {
router = TestBed.inject(Router);
location = TestBed.inject(Location);
fixture = TestBed.createComponent(TestHostComponent);
testHostComponent = fixture.componentInstance;
testHostComponent = harness.fixture.debugElement.query(By.directive(TestHostComponent)).componentInstance as TestHostComponent;
anchorButton = testHostComponent.anchor.nativeElement;
fixture.detectChanges();
tick();
processUpdates();
innerAnchor = anchorButton!.shadowRoot!.querySelector('a')!;
routerNavigateByUrlSpy = spyOn(router, 'navigateByUrl').and.callThrough();
anchorClickHandlerSpy = jasmine.createSpy('click');
anchorClickHandlerSpy = jasmine.createSpy('click').and.callFake((event: Event) => event.preventDefault());
innerAnchor!.addEventListener('click', anchorClickHandlerSpy);
}));
});

afterEach(() => {
processUpdates();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Component } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { CommonModule } from '@angular/common';
import { RouterTestingModule } from '@angular/router/testing';
import { processUpdates } from '../../../testing/async-helpers';
import { NimbleAnchorButtonModule } from '../nimble-anchor-button.module';

Expand All @@ -23,8 +22,7 @@ describe('Nimble anchor button RouterLinkDirective', () => {
imports: [
NimbleAnchorButtonModule,
CommonModule,
RouterTestingModule.withRoutes([{ path: '', component: TestHostComponent, pathMatch: 'full' }], { useHash: true })
]
],
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Component, ElementRef, Sanitizer, SecurityContext, ViewChild } from '@angular/core';
import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
import { Router } from '@angular/router';
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { provideRouter, Router } from '@angular/router';
import { CommonModule, Location } from '@angular/common';
import { RouterTestingModule } from '@angular/router/testing';
import { RouterTestingHarness } from '@angular/router/testing';
import { parameterizeSpec } from '@ni/jasmine-parameterized';
import { processUpdates } from '../../../testing/async-helpers';
import { NimbleAnchorMenuItemModule } from '../nimble-anchor-menu-item.module';
Expand All @@ -14,7 +15,6 @@ describe('Nimble anchor menu item RouterLinkWithHrefDirective', () => {
<nimble-anchor-menu-item #menuItem nimbleRouterLink="page1" [queryParams]="{param1: true}" [state]="{stateProperty: 123}">
Anchor text
</nimble-anchor-menu-item>
<router-outlet></router-outlet>
`
})
class TestHostComponent {
Expand All @@ -25,49 +25,47 @@ describe('Nimble anchor menu item RouterLinkWithHrefDirective', () => {
class BlankComponent { }

let menuItem: AnchorMenuItem;
let fixture: ComponentFixture<TestHostComponent>;
let testHostComponent: TestHostComponent;
let router: Router;
let location: Location;
let innerAnchor: HTMLAnchorElement;
let routerNavigateByUrlSpy: jasmine.Spy;
let anchorClickHandlerSpy: jasmine.Spy;
let sanitizer: jasmine.SpyObj<Sanitizer>;
let harness: RouterTestingHarness;

beforeEach(() => {
beforeEach(async () => {
sanitizer = jasmine.createSpyObj<Sanitizer>('Sanitizer', ['sanitize']);
sanitizer.sanitize.and.callFake((_, value: string) => value);

TestBed.configureTestingModule({
declarations: [TestHostComponent, BlankComponent],
imports: [NimbleAnchorMenuItemModule,
imports: [
NimbleAnchorMenuItemModule,
CommonModule,
RouterTestingModule.withRoutes([
{ path: '', redirectTo: '/start', pathMatch: 'full' },
{ path: 'page1', component: BlankComponent },
{ path: 'start', component: TestHostComponent }
], { useHash: true })
],
providers: [
{ provide: Sanitizer, useValue: sanitizer }
{ provide: Sanitizer, useValue: sanitizer },
provideRouter([
{ path: 'page1', component: BlankComponent },
{ path: '', component: TestHostComponent }
]),
]
});
harness = await RouterTestingHarness.create('');
});

beforeEach(fakeAsync(() => {
beforeEach(() => {
router = TestBed.inject(Router);
location = TestBed.inject(Location);
fixture = TestBed.createComponent(TestHostComponent);
testHostComponent = fixture.componentInstance;
testHostComponent = harness.fixture.debugElement.query(By.directive(TestHostComponent)).componentInstance as TestHostComponent;
menuItem = testHostComponent.menuItem.nativeElement;
fixture.detectChanges();
tick();
processUpdates();
innerAnchor = menuItem!.shadowRoot!.querySelector('a')!;
routerNavigateByUrlSpy = spyOn(router, 'navigateByUrl').and.callThrough();
anchorClickHandlerSpy = jasmine.createSpy('click');
anchorClickHandlerSpy = jasmine.createSpy('click').and.callFake((event: Event) => event.preventDefault());
innerAnchor!.addEventListener('click', anchorClickHandlerSpy);
}));
});

afterEach(() => {
processUpdates();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Component } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { CommonModule } from '@angular/common';
import { RouterTestingModule } from '@angular/router/testing';
import { processUpdates } from '../../../testing/async-helpers';
import { NimbleAnchorMenuItemModule } from '../nimble-anchor-menu-item.module';

Expand All @@ -20,11 +19,10 @@ describe('Nimble anchor menu item RouterLinkDirective', () => {
beforeEach(() => {
TestBed.configureTestingModule({
declarations: [TestHostComponent],
imports: [NimbleAnchorMenuItemModule,
CommonModule,
RouterTestingModule.withRoutes([{ path: '', component: TestHostComponent, pathMatch: 'full' }
], { useHash: true })
]
imports: [
NimbleAnchorMenuItemModule,
CommonModule
],
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Component, ElementRef, Sanitizer, SecurityContext, ViewChild } from '@angular/core';
import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
import { Router } from '@angular/router';
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
import { provideRouter, Router } from '@angular/router';
import { CommonModule, Location } from '@angular/common';
import { RouterTestingModule } from '@angular/router/testing';
import { By } from '@angular/platform-browser';
import { RouterTestingHarness } from '@angular/router/testing';
import { parameterizeSpec } from '@ni/jasmine-parameterized';
import { processUpdates } from '../../../testing/async-helpers';
import { NimbleAnchorTabModule } from '../nimble-anchor-tab.module';
Expand All @@ -14,7 +15,6 @@ describe('Nimble anchor tab RouterLinkWithHrefDirective', () => {
<nimble-anchor-tab #anchorTab nimbleRouterLink="page1" [queryParams]="{param1: true}" [state]="{stateProperty: 123}">
Anchor text
</nimble-anchor-tab>
<router-outlet></router-outlet>
`
})
class TestHostComponent {
Expand All @@ -25,49 +25,47 @@ describe('Nimble anchor tab RouterLinkWithHrefDirective', () => {
class BlankComponent { }

let anchorTab: AnchorTab;
let fixture: ComponentFixture<TestHostComponent>;
let testHostComponent: TestHostComponent;
let router: Router;
let location: Location;
let innerAnchor: HTMLAnchorElement;
let routerNavigateByUrlSpy: jasmine.Spy;
let anchorClickHandlerSpy: jasmine.Spy;
let sanitizer: jasmine.SpyObj<Sanitizer>;
let harness: RouterTestingHarness;

beforeEach(() => {
beforeEach(async () => {
sanitizer = jasmine.createSpyObj<Sanitizer>('Sanitizer', ['sanitize']);
sanitizer.sanitize.and.callFake((_, value: string) => value);

TestBed.configureTestingModule({
declarations: [TestHostComponent, BlankComponent],
imports: [NimbleAnchorTabModule,
imports: [
NimbleAnchorTabModule,
CommonModule,
RouterTestingModule.withRoutes([
{ path: '', redirectTo: '/start', pathMatch: 'full' },
{ path: 'page1', component: BlankComponent },
{ path: 'start', component: TestHostComponent }
], { useHash: true })
],
providers: [
{ provide: Sanitizer, useValue: sanitizer }
{ provide: Sanitizer, useValue: sanitizer },
provideRouter([
{ path: 'page1', component: BlankComponent },
{ path: '', component: TestHostComponent }
]),
]
});
harness = await RouterTestingHarness.create('');
});

beforeEach(fakeAsync(() => {
beforeEach(() => {
router = TestBed.inject(Router);
location = TestBed.inject(Location);
fixture = TestBed.createComponent(TestHostComponent);
testHostComponent = fixture.componentInstance;
testHostComponent = harness.fixture.debugElement.query(By.directive(TestHostComponent)).componentInstance as TestHostComponent;
anchorTab = testHostComponent.anchorTab.nativeElement;
fixture.detectChanges();
tick();
processUpdates();
innerAnchor = anchorTab!.shadowRoot!.querySelector('a')!;
routerNavigateByUrlSpy = spyOn(router, 'navigateByUrl').and.callThrough();
anchorClickHandlerSpy = jasmine.createSpy('click');
anchorClickHandlerSpy = jasmine.createSpy('click').and.callFake((event: Event) => event.preventDefault());
innerAnchor!.addEventListener('click', anchorClickHandlerSpy);
}));
});

afterEach(() => {
processUpdates();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Component } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { CommonModule } from '@angular/common';
import { RouterTestingModule } from '@angular/router/testing';
import { processUpdates } from '../../../testing/async-helpers';
import { NimbleAnchorTabModule } from '../nimble-anchor-tab.module';

Expand All @@ -20,12 +19,10 @@ describe('Nimble anchor tab RouterLinkDirective', () => {
beforeEach(() => {
TestBed.configureTestingModule({
declarations: [TestHostComponent],
imports: [NimbleAnchorTabModule,
imports: [
NimbleAnchorTabModule,
CommonModule,
RouterTestingModule.withRoutes([
{ path: '', component: TestHostComponent, pathMatch: 'full' }
], { useHash: true })
]
],
});
});

Expand Down
Loading

0 comments on commit 943ce38

Please sign in to comment.