ocket8888 commented on code in PR #7881: URL: https://github.com/apache/trafficcontrol/pull/7881#discussion_r1471588991
########## experimental/traffic-portal/src/app/core/origins/detail/origin-detail.component.spec.ts: ########## @@ -0,0 +1,124 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { HarnessLoader } from "@angular/cdk/testing"; +import { TestbedHarnessEnvironment } from "@angular/cdk/testing/testbed"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { MatButtonHarness } from "@angular/material/button/testing"; +import { MatDialogModule } from "@angular/material/dialog"; +import { MatDialogHarness } from "@angular/material/dialog/testing"; +import { ActivatedRoute } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { ReplaySubject } from "rxjs"; + +import { OriginService } from "src/app/api"; +import { APITestingModule } from "src/app/api/testing"; +import { NavigationService } from "src/app/shared/navigation/navigation.service"; + +import { OriginDetailComponent } from "./origin-detail.component"; + +describe("OriginDetailComponent", () => { + let component: OriginDetailComponent; + let fixture: ComponentFixture<OriginDetailComponent>; + let route: ActivatedRoute; + let paramMap: jasmine.Spy; + let loader: HarnessLoader; + let service: OriginService; + + const navSvc = jasmine.createSpyObj([], { + headerHidden: new ReplaySubject<boolean>(), + headerTitle: new ReplaySubject<string>(), + }); + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [OriginDetailComponent], + imports: [APITestingModule, RouterTestingModule, MatDialogModule], Review Comment: The reason your tests are failing is because the dialog never actually closes in time for the tests, and things waiting for it to close never happen. The problem is you're waiting on animations, but without explicitly providing an animations module. As you can see in [the testing example in the Angular Material Dialog component documentation](https://material.angular.io/components/dialog/examples#dialog-harness), you should provide the `NoopAnimationsModule` from `@angular/platform-browser/animations`. ########## experimental/traffic-portal/src/app/core/origins/detail/origin-detail.component.spec.ts: ########## @@ -0,0 +1,124 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { HarnessLoader } from "@angular/cdk/testing"; +import { TestbedHarnessEnvironment } from "@angular/cdk/testing/testbed"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { MatButtonHarness } from "@angular/material/button/testing"; +import { MatDialogModule } from "@angular/material/dialog"; +import { MatDialogHarness } from "@angular/material/dialog/testing"; +import { ActivatedRoute } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { ReplaySubject } from "rxjs"; + +import { OriginService } from "src/app/api"; +import { APITestingModule } from "src/app/api/testing"; +import { NavigationService } from "src/app/shared/navigation/navigation.service"; + +import { OriginDetailComponent } from "./origin-detail.component"; + +describe("OriginDetailComponent", () => { + let component: OriginDetailComponent; + let fixture: ComponentFixture<OriginDetailComponent>; + let route: ActivatedRoute; + let paramMap: jasmine.Spy; + let loader: HarnessLoader; + let service: OriginService; + + const navSvc = jasmine.createSpyObj([], { + headerHidden: new ReplaySubject<boolean>(), + headerTitle: new ReplaySubject<string>(), + }); + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [OriginDetailComponent], + imports: [APITestingModule, RouterTestingModule, MatDialogModule], + providers: [{provide: NavigationService, useValue: navSvc}], + }).compileComponents(); + + route = TestBed.inject(ActivatedRoute); + paramMap = spyOn(route.snapshot.paramMap, "get"); + paramMap.and.returnValue(null); + fixture = TestBed.createComponent(OriginDetailComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + service = TestBed.inject(OriginService); + loader = TestbedHarnessEnvironment.documentRootLoader(fixture); + await fixture.whenStable(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); + + it("new origin", async () => { + paramMap.and.returnValue("new"); + + fixture = TestBed.createComponent(OriginDetailComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + await fixture.whenStable(); + expect(paramMap).toHaveBeenCalled(); + expect(component.origin).not.toBeNull(); + expect(component.origin.name).toBe(""); + expect(component.new).toBeTrue(); + }); + + it("existing origin", async () => { + const id = 1; + paramMap.and.returnValue(id); + const origin = await service.getOrigins(id); + fixture = TestBed.createComponent(OriginDetailComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + await fixture.whenStable(); + expect(paramMap).toHaveBeenCalled(); + expect(component.origin).not.toBeNull(); + expect(component.origin.name).toBe(origin.name); + expect(component.new).toBeFalse(); + }); + + it("deletes existing Origins", async () => { + const spy = spyOn(service, "deleteOrigin").and.callThrough(); + let cgs = await service.getOrigins(); + const initialLength = cgs.length; + if (initialLength < 1) { + return fail("need at least one Cache Group"); + } + const cg = cgs[0]; + component.origin = cg; + component.new = false; + + const asyncExpectation = expectAsync(component.deleteOrigin()).toBeResolvedTo(undefined); + const dialogs = await loader.getAllHarnesses(MatDialogHarness); + if (dialogs.length !== 1) { + return fail(`failed to open dialog; ${dialogs.length} dialogs found`); + } + const dialog = dialogs[0]; + const buttons = await dialog.getAllHarnesses(MatButtonHarness.with({text: /^[cC][oO][nN][fF][iI][rR][mM]$/})); + if (buttons.length !== 1) { + return fail(`'confirm' button not found; ${buttons.length} buttons found`); + } + await buttons[0].click(); + + expect(spy).toHaveBeenCalledOnceWith(cg); Review Comment: You're explicitly calling `deleteOrigin` with the **ID** of the Origin being deleted, not the entire Origin. So once you fix the testing setup, this will fail, because it'll expect a call with a whole origin passed but you're passing a number. ########## experimental/traffic-portal/src/app/core/origins/detail/origin-detail.component.spec.ts: ########## @@ -0,0 +1,124 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { HarnessLoader } from "@angular/cdk/testing"; +import { TestbedHarnessEnvironment } from "@angular/cdk/testing/testbed"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { MatButtonHarness } from "@angular/material/button/testing"; +import { MatDialogModule } from "@angular/material/dialog"; +import { MatDialogHarness } from "@angular/material/dialog/testing"; +import { ActivatedRoute } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { ReplaySubject } from "rxjs"; + +import { OriginService } from "src/app/api"; +import { APITestingModule } from "src/app/api/testing"; +import { NavigationService } from "src/app/shared/navigation/navigation.service"; + +import { OriginDetailComponent } from "./origin-detail.component"; + +describe("OriginDetailComponent", () => { + let component: OriginDetailComponent; + let fixture: ComponentFixture<OriginDetailComponent>; + let route: ActivatedRoute; + let paramMap: jasmine.Spy; + let loader: HarnessLoader; + let service: OriginService; + + const navSvc = jasmine.createSpyObj([], { + headerHidden: new ReplaySubject<boolean>(), + headerTitle: new ReplaySubject<string>(), + }); + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [OriginDetailComponent], + imports: [APITestingModule, RouterTestingModule, MatDialogModule], + providers: [{provide: NavigationService, useValue: navSvc}], + }).compileComponents(); + + route = TestBed.inject(ActivatedRoute); + paramMap = spyOn(route.snapshot.paramMap, "get"); + paramMap.and.returnValue(null); + fixture = TestBed.createComponent(OriginDetailComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + service = TestBed.inject(OriginService); + loader = TestbedHarnessEnvironment.documentRootLoader(fixture); + await fixture.whenStable(); + }); + + it("should create", () => { + expect(component).toBeTruthy(); + }); + + it("new origin", async () => { + paramMap.and.returnValue("new"); + + fixture = TestBed.createComponent(OriginDetailComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + await fixture.whenStable(); + expect(paramMap).toHaveBeenCalled(); + expect(component.origin).not.toBeNull(); + expect(component.origin.name).toBe(""); + expect(component.new).toBeTrue(); + }); + + it("existing origin", async () => { + const id = 1; + paramMap.and.returnValue(id); + const origin = await service.getOrigins(id); + fixture = TestBed.createComponent(OriginDetailComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + await fixture.whenStable(); + expect(paramMap).toHaveBeenCalled(); + expect(component.origin).not.toBeNull(); + expect(component.origin.name).toBe(origin.name); + expect(component.new).toBeFalse(); + }); + + it("deletes existing Origins", async () => { + const spy = spyOn(service, "deleteOrigin").and.callThrough(); + let cgs = await service.getOrigins(); + const initialLength = cgs.length; + if (initialLength < 1) { + return fail("need at least one Cache Group"); Review Comment: You actually just need at least one _Origin_ Also, names of things like `cgs` are clearly stale copypasta from the Cache Group details page spec - should probably be e.g. `orgs`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
