You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/05/03 13:30:41 UTC

[GitHub] [trafficcontrol] shamrickus opened a new pull request, #6805: Add Various E2E TPv2 DS Tests

shamrickus opened a new pull request, #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805

   <!--
   Thank you for contributing! Please be sure to read our contribution guidelines: https://github.com/apache/trafficcontrol/blob/master/CONTRIBUTING.md
   If this closes or relates to an existing issue, please reference it using one of the following:
   
   Closes: #ISSUE
   Related: #ISSUE
   
   If this PR fixes a security vulnerability, DO NOT submit! Instead, contact
   the Apache Traffic Control Security Team at security@trafficcontrol.apache.org and follow the
   guidelines at https://apache.org/security regarding vulnerability disclosure.
   -->
   
   This PR creates E2E tests for DS card, details, and invalidation jobs. It also adds a relatively simple api client implementation to create a DS used for testing. Tests have been moved into folders to allow for [test groups](https://nightwatchjs.org/guide/running-tests/test-groups.html). Fail fast has been enabled for non ci; however, it doesn't appear to actually fail that much faster.  Added a npm script to clean up TPv2, very useful when switching between branches. Finally the test have been refactored to use mocha style testing.
   <!-- **^ Add meaningful description above** --><hr/>
   
   ## Which Traffic Control components are affected by this PR?
   <!-- Please delete all components from this list that are NOT affected by this PR.
   Feel free to add the name of a tool or script that is affected but not on the list.
   -->
   - Traffic Portal v2
   
   ## What is the best way to verify this PR?
   Verify the tests pass and have adequate coverage.
   <!-- Please include here ALL the steps necessary to test your PR.
   If your PR has tests (and most should), provide the steps needed to run the tests.
   If not, please provide step-by-step instructions to test the PR manually and explain why your PR does not need tests. -->
   
   <!-- Delete this section if the PR is not a bugfix, or if the bug is only in the master branch.
   Examples:
   - 5.1.2
   - 5.1.3 (RC1)
    -->
   
   
   ## PR submission checklist
   - [x] This PR has tests <!-- If not, please delete this text and explain why this PR does not need tests. -->
   - [ ] This PR has documentation <!-- If not, please delete this text and explain why this PR does not need documentation. -->
   - [ ] This PR has a CHANGELOG.md entry <!-- A fix for a bug from an ATC release, an improvement, or a new feature should have a changelog entry. -->
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://apache.org/security) for details)
   
   <!--
   Licensed to the Apache Software Foundation (ASF) under one
   or more contributor license agreements.  See the NOTICE file
   distributed with this work for additional information
   regarding copyright ownership.  The ASF licenses this file
   to you 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.
   -->
   


-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
shamrickus commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r867255636


##########
experimental/traffic-portal/nightwatch/page_objects/deliveryServiceCard.ts:
##########
@@ -0,0 +1,77 @@
+/*
+* 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 {
+	EnhancedElementInstance,
+	EnhancedPageObject, EnhancedSectionInstance, NightwatchAPI
+} from "nightwatch";
+
+/**
+ * Defines the commands for the loginForm section
+ */
+interface DeliveryServiceCardCommands extends EnhancedSectionInstance, EnhancedElementInstance<EnhancedPageObject> {
+	expandDS(xmlId: string): Promise<boolean>;
+	viewDetails(xmlId: string): Promise<boolean>;
+}
+
+/**
+ * Defines the loginForm section
+ */
+type DeliveryServiceCardSection = EnhancedSectionInstance<DeliveryServiceCardCommands,
+	typeof deliveryServiceCardPageObject.sections.cards.elements>;
+
+/**
+ * Define the type for our PO
+ */
+export type DeliveryServiceCardPageObject = EnhancedPageObject<{}, {}, { cards: DeliveryServiceCardSection }>;
+
+const deliveryServiceCardPageObject = {
+	api: {} as NightwatchAPI,
+	sections: {
+		cards: {
+			commands: {
+				async expandDS(xmlId: string): Promise<boolean> {
+					return new Promise((resolve, reject) => {
+						this.click("css selector", `mat-card#${xmlId}`, result => {
+							if (result.status === 1) {
+								reject(new Error(`Unable to find by css mat-card#${xmlId}`));
+								return;
+							}
+							this.waitForElementVisible(`mat-card#${xmlId} mat-card-content > div`,
+								undefined, undefined, undefined, () => {
+									resolve(true);
+								});
+						});
+					});
+				},
+				async viewDetails(xmlId: string): Promise<boolean> {
+					await this.expandDS(xmlId);
+					return new Promise((resolve) => {
+						this.click("css selector", `mat-card#${xmlId} mat-card-actions > a`, () => {
+							browser.assert.urlContains("deliveryservice");

Review Comment:
   No, but it shouldn't matter because test execution halts. The logs show the relevant error in the correct position.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r868428754


##########
experimental/traffic-portal/nightwatch/globals/globals.ts:
##########
@@ -21,11 +53,97 @@ export interface GlobalConfig extends NightwatchGlobals {
 	adminPass: string;
 	adminUser: string;
 	trafficOpsURL: string;
+	apiVersion: string;
 }
-const config = {
+const globals = {
 	adminPass: "twelve12",
 	adminUser: "admin",
+	apiVersion: "4.0",
+	before: async (done: () => void): Promise<void> => {
+		const apiUrl = `${globals.trafficOpsURL}/api/${globals.apiVersion}`;
+		const client = axios.create({
+			httpsAgent: new https.Agent({
+				rejectUnauthorized: false
+			})
+		});
+		let accessToken = "";
+		const loginReq: LoginRequest = {
+			p: globals.adminPass,
+			u: globals.adminUser
+		};
+		try {
+			const resp = await client.post(`${apiUrl}/user/login`, JSON.stringify(loginReq));
+			if(resp.headers["set-cookie"]) {
+				for (const cookie of resp.headers["set-cookie"]) {
+					if(cookie.indexOf("access_token") > -1) {
+						accessToken = cookie;
+						break;
+					}
+				}
+			}
+		} catch (e) {
+			console.error((e as AxiosError).message);
+			return Promise.reject();

Review Comment:
   > without the manual log, nothing is output and the process exits
   
   That's truly insane



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r868429991


##########
experimental/traffic-portal/nightwatch/tests/ds/ds.card.ts:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 {type AugmentedBrowser} from "nightwatch/globals/globals";
+
+describe("DS Card Spec", () => {
+	let augBrowser: AugmentedBrowser;
+	before(() => {
+		augBrowser = browser as AugmentedBrowser;
+	});
+
+	beforeEach(() => {
+		augBrowser.page.login()
+			.navigate().section.loginForm
+			.loginAndWait(augBrowser.globals.adminUser, augBrowser.globals.adminPass);
+	});

Review Comment:
   Well between each `describe` I'd understand since it appears to re-initialize the browser, but each `it` clears the session? That's strange.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
shamrickus commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r867258845


##########
experimental/traffic-portal/nightwatch/tests/ds/ds.card.ts:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 {type AugmentedBrowser} from "nightwatch/globals/globals";
+
+describe("DS Card Spec", () => {
+	let augBrowser: AugmentedBrowser;
+	before(() => {
+		augBrowser = browser as AugmentedBrowser;
+	});
+
+	beforeEach(() => {
+		augBrowser.page.login()
+			.navigate().section.loginForm
+			.loginAndWait(augBrowser.globals.adminUser, augBrowser.globals.adminPass);
+	});

Review Comment:
   No, it appears cookies are cleared (or something equivalent) between test runs.



##########
experimental/traffic-portal/nightwatch/tests/ds/ds.card.ts:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 {type AugmentedBrowser} from "nightwatch/globals/globals";
+
+describe("DS Card Spec", () => {
+	let augBrowser: AugmentedBrowser;
+	before(() => {
+		augBrowser = browser as AugmentedBrowser;
+	});
+
+	beforeEach(() => {
+		augBrowser.page.login()
+			.navigate().section.loginForm
+			.loginAndWait(augBrowser.globals.adminUser, augBrowser.globals.adminPass);
+	});

Review Comment:
   Yes, it appears cookies are cleared (or something equivalent) between test runs.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
shamrickus commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r869321013


##########
experimental/traffic-portal/nightwatch/tests/ds/ds.card.ts:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 {type AugmentedBrowser} from "nightwatch/globals/globals";
+
+describe("DS Card Spec", () => {
+	let augBrowser: AugmentedBrowser;
+	before(() => {
+		augBrowser = browser as AugmentedBrowser;
+	});
+
+	beforeEach(() => {
+		augBrowser.page.login()
+			.navigate().section.loginForm
+			.loginAndWait(augBrowser.globals.adminUser, augBrowser.globals.adminPass);
+	});

Review Comment:
   Actually it appears we cannot add cookies due to security settings that are enforced on cookies. That having been said, I did move the login code to the `globals` so each test does not need to worry about that.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r864221318


##########
experimental/traffic-portal/src/app/core/ds-card/ds-card.component.html:
##########
@@ -57,7 +57,7 @@ <h3 *ngIf="deliveryService.exampleURLs">URLs</h3>
 		</div>
 	</mat-card-content>
 	<mat-card-actions *ngIf="open">
-		<a mat-raised-button color="accent" class="view-details" routerLink="/core/deliveryservice/{{deliveryService.id}}" title="View Details" aria-label="View Details">
+		<a id="{{deliveryService.xmlId}}Details" mat-raised-button color="accent" class="view-details" routerLink="/core/deliveryservice/{{deliveryService.id}}" title="View Details" aria-label="View Details">

Review Comment:
   I have a similar complaint here; the equivalent selector being `mat-card#XMLID mat-card-actions > a`.



##########
experimental/traffic-portal/src/app/core/ds-card/ds-card.component.html:
##########
@@ -11,15 +11,15 @@
 See the License for the specific language governing permissions and
 limitations under the License.
 -->
-<mat-card [ngClass]="{'inactive': !deliveryService.active, 'open': open, 'first': first, 'last': last}">
+<mat-card id="{{deliveryService.xmlId}}" [ngClass]="{'inactive': !deliveryService.active, 'open': open, 'first': first, 'last': last}">
 	<mat-card-title-group (click)="toggle()">
 		<mat-card-title>{{deliveryService.displayName}}{{deliveryService.active ? '' : ' (inactive)'}}
 			<a href="{{deliveryService.infoUrl}}" *ngIf="deliveryService.infoUrl" class="color-accent-inverted info" rel="noopener" target="_blank" title="More Information"><fa-icon [icon]="infoIcon"></fa-icon></a>
 		</mat-card-title>
 		<mat-card-subtitle>{{deliveryService.type ? deliveryService.type : ''}}</mat-card-subtitle>
 	</mat-card-title-group>
 	<mat-card-content class="card-content" *ngIf="open" [@enterAnimation]>
-		<div>
+		<div id="{{deliveryService.xmlId}}Content">

Review Comment:
   I don't feel good about adding classes and IDs just for testing to look for - I think the card having an ID overall is actually kinda a good idea in general so that maybe people could link to a specific DS (although I imagine in general they'd link to a page where the search filters down to just that DS, but still) but this one doesn't seem to have any utility beyond just being shorter to type than `mat-card#XMLID mat-card-content > div`.



##########
experimental/traffic-portal/nightwatch/tests/_bootstrap.spec.ts:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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 * as https from "https";
+
+import axios from "axios";
+import {AugmentedBrowser} from "nightwatch/globals/globals";
+import {
+	CDN,
+	GeoLimit, GeoProvider, LoginRequest,
+	Protocol,
+	RequestDeliveryService,
+	ResponseCDN,
+	ResponseDeliveryService
+} from "trafficops-types";
+
+/**
+ * Creates data necessary for the e2e tests, currently just a CDN, and a DS
+ *
+ * @param augBrowser The browser object
+ * @returns Created Delivery Service
+ */
+async function createData(augBrowser: AugmentedBrowser): Promise<ResponseDeliveryService> {
+	const apiUrl = `${augBrowser.globals.trafficOpsURL}/api/${augBrowser.globals.apiVersion}`;
+	const client = axios.create({
+		httpsAgent: new https.Agent({
+			rejectUnauthorized: false
+		})
+	});
+	let accessToken = "";
+	const loginReq: LoginRequest = {
+		p: augBrowser.globals.adminPass,
+		u: augBrowser.globals.adminUser
+	};
+	try {
+		const resp = await client.post(`${apiUrl}/user/login`, JSON.stringify(loginReq));
+		if(resp.headers["set-cookie"]) {
+			for (const cookie of resp.headers["set-cookie"]) {
+				if(cookie.indexOf("access_token") > -1) {
+					accessToken = cookie;
+					break;
+				}
+			}
+		}
+	} catch (e) {
+		return Promise.reject(e);
+	}
+	if(accessToken === "") {
+		return Promise.reject("Access token is not set");
+	}
+	client.defaults.headers.common = { Cookie: accessToken };
+
+	const cdn: CDN = {
+		dnssecEnabled: false, domainName: "tests.com", name: "testCDN"
+	};
+	let respCDN: ResponseCDN;
+	try {
+		let resp = await client.post(`${apiUrl}/cdns`, JSON.stringify(cdn));
+		respCDN = resp.data.response;
+
+		const ds: RequestDeliveryService = {
+			active: false,
+			cacheurl: null,
+			cdnId: respCDN.id,
+			displayName: "test DS",
+			dscp: 0,
+			ecsEnabled: false,
+			edgeHeaderRewrite: null,
+			fqPacingRate: null,
+			geoLimit: GeoLimit.NONE,
+			geoProvider: GeoProvider.MAX_MIND,
+			httpBypassFqdn: null,
+			infoUrl: null,
+			initialDispersion: 1,
+			ipv6RoutingEnabled: false,
+			logsEnabled: false,
+			maxOriginConnections: 0,
+			maxRequestHeaderBytes: 0,
+			midHeaderRewrite: null,
+			missLat: 0,
+			missLong: 0,
+			multiSiteOrigin: false,
+			orgServerFqdn: "http://test.com",
+			profileId: 1,
+			protocol: Protocol.HTTP,
+			qstringIgnore: 0,
+			rangeRequestHandling: 0,
+			regionalGeoBlocking: false,
+			remapText: null,
+			routingName: "test",
+			signed: false,
+			tenantId: 1,
+			typeId: 1,
+			xmlId: "testDS"
+		};
+		resp = await client.post(`${apiUrl}/deliveryservices`, JSON.stringify(ds));
+		const respDS: ResponseDeliveryService = resp.data.response[0];
+
+		return Promise.resolve(respDS);
+	} catch(e) {
+		return Promise.reject(e);
+	}
+}
+
+describe("Bootstrap Spec", () => {
+	it("Create Data", async (): Promise<void> => {
+		const augBrowser = browser as AugmentedBrowser;
+		await createData(augBrowser).then(ds => {
+			console.log(`Successfully created DS '${ds.displayName}'`);
+		}).catch(err => {
+			augBrowser.assert.fail(err, null, "Exception occurred while creating data");
+		}).finally(() => {
+			augBrowser.end();
+		});
+	});
+});

Review Comment:
   Should this be in nightwatch's global `before` to run before all specs?



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
shamrickus commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r867255636


##########
experimental/traffic-portal/nightwatch/page_objects/deliveryServiceCard.ts:
##########
@@ -0,0 +1,77 @@
+/*
+* 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 {
+	EnhancedElementInstance,
+	EnhancedPageObject, EnhancedSectionInstance, NightwatchAPI
+} from "nightwatch";
+
+/**
+ * Defines the commands for the loginForm section
+ */
+interface DeliveryServiceCardCommands extends EnhancedSectionInstance, EnhancedElementInstance<EnhancedPageObject> {
+	expandDS(xmlId: string): Promise<boolean>;
+	viewDetails(xmlId: string): Promise<boolean>;
+}
+
+/**
+ * Defines the loginForm section
+ */
+type DeliveryServiceCardSection = EnhancedSectionInstance<DeliveryServiceCardCommands,
+	typeof deliveryServiceCardPageObject.sections.cards.elements>;
+
+/**
+ * Define the type for our PO
+ */
+export type DeliveryServiceCardPageObject = EnhancedPageObject<{}, {}, { cards: DeliveryServiceCardSection }>;
+
+const deliveryServiceCardPageObject = {
+	api: {} as NightwatchAPI,
+	sections: {
+		cards: {
+			commands: {
+				async expandDS(xmlId: string): Promise<boolean> {
+					return new Promise((resolve, reject) => {
+						this.click("css selector", `mat-card#${xmlId}`, result => {
+							if (result.status === 1) {
+								reject(new Error(`Unable to find by css mat-card#${xmlId}`));
+								return;
+							}
+							this.waitForElementVisible(`mat-card#${xmlId} mat-card-content > div`,
+								undefined, undefined, undefined, () => {
+									resolve(true);
+								});
+						});
+					});
+				},
+				async viewDetails(xmlId: string): Promise<boolean> {
+					await this.expandDS(xmlId);
+					return new Promise((resolve) => {
+						this.click("css selector", `mat-card#${xmlId} mat-card-actions > a`, () => {
+							browser.assert.urlContains("deliveryservice");

Review Comment:
   Test execution halts so no, but it doesn't matter. The logs show the relevant error in the relevant position.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
shamrickus commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r869292619


##########
experimental/traffic-portal/nightwatch/tests/ds/ds.card.ts:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 {type AugmentedBrowser} from "nightwatch/globals/globals";
+
+describe("DS Card Spec", () => {
+	let augBrowser: AugmentedBrowser;
+	before(() => {
+		augBrowser = browser as AugmentedBrowser;
+	});
+
+	beforeEach(() => {
+		augBrowser.page.login()
+			.navigate().section.loginForm
+			.loginAndWait(augBrowser.globals.adminUser, augBrowser.globals.adminPass);
+	});

Review Comment:
   The browser session is closed whenever `.end` is called. We don't need to be calling that between each spec, but probably between suites. I'll also add some code to cache the `access_token` so the login endpoint doesn't actually need to be called everytime.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
shamrickus commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r864234642


##########
experimental/traffic-portal/nightwatch/tests/_bootstrap.spec.ts:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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 * as https from "https";
+
+import axios from "axios";
+import {AugmentedBrowser} from "nightwatch/globals/globals";
+import {
+	CDN,
+	GeoLimit, GeoProvider, LoginRequest,
+	Protocol,
+	RequestDeliveryService,
+	ResponseCDN,
+	ResponseDeliveryService
+} from "trafficops-types";
+
+/**
+ * Creates data necessary for the e2e tests, currently just a CDN, and a DS
+ *
+ * @param augBrowser The browser object
+ * @returns Created Delivery Service
+ */
+async function createData(augBrowser: AugmentedBrowser): Promise<ResponseDeliveryService> {
+	const apiUrl = `${augBrowser.globals.trafficOpsURL}/api/${augBrowser.globals.apiVersion}`;
+	const client = axios.create({
+		httpsAgent: new https.Agent({
+			rejectUnauthorized: false
+		})
+	});
+	let accessToken = "";
+	const loginReq: LoginRequest = {
+		p: augBrowser.globals.adminPass,
+		u: augBrowser.globals.adminUser
+	};
+	try {
+		const resp = await client.post(`${apiUrl}/user/login`, JSON.stringify(loginReq));
+		if(resp.headers["set-cookie"]) {
+			for (const cookie of resp.headers["set-cookie"]) {
+				if(cookie.indexOf("access_token") > -1) {
+					accessToken = cookie;
+					break;
+				}
+			}
+		}
+	} catch (e) {
+		return Promise.reject(e);
+	}
+	if(accessToken === "") {
+		return Promise.reject("Access token is not set");
+	}
+	client.defaults.headers.common = { Cookie: accessToken };
+
+	const cdn: CDN = {
+		dnssecEnabled: false, domainName: "tests.com", name: "testCDN"
+	};
+	let respCDN: ResponseCDN;
+	try {
+		let resp = await client.post(`${apiUrl}/cdns`, JSON.stringify(cdn));
+		respCDN = resp.data.response;
+
+		const ds: RequestDeliveryService = {
+			active: false,
+			cacheurl: null,
+			cdnId: respCDN.id,
+			displayName: "test DS",
+			dscp: 0,
+			ecsEnabled: false,
+			edgeHeaderRewrite: null,
+			fqPacingRate: null,
+			geoLimit: GeoLimit.NONE,
+			geoProvider: GeoProvider.MAX_MIND,
+			httpBypassFqdn: null,
+			infoUrl: null,
+			initialDispersion: 1,
+			ipv6RoutingEnabled: false,
+			logsEnabled: false,
+			maxOriginConnections: 0,
+			maxRequestHeaderBytes: 0,
+			midHeaderRewrite: null,
+			missLat: 0,
+			missLong: 0,
+			multiSiteOrigin: false,
+			orgServerFqdn: "http://test.com",
+			profileId: 1,
+			protocol: Protocol.HTTP,
+			qstringIgnore: 0,
+			rangeRequestHandling: 0,
+			regionalGeoBlocking: false,
+			remapText: null,
+			routingName: "test",
+			signed: false,
+			tenantId: 1,
+			typeId: 1,
+			xmlId: "testDS"
+		};
+		resp = await client.post(`${apiUrl}/deliveryservices`, JSON.stringify(ds));
+		const respDS: ResponseDeliveryService = resp.data.response[0];
+
+		return Promise.resolve(respDS);
+	} catch(e) {
+		return Promise.reject(e);
+	}
+}
+
+describe("Bootstrap Spec", () => {
+	it("Create Data", async (): Promise<void> => {
+		const augBrowser = browser as AugmentedBrowser;
+		await createData(augBrowser).then(ds => {
+			console.log(`Successfully created DS '${ds.displayName}'`);
+		}).catch(err => {
+			augBrowser.assert.fail(err, null, "Exception occurred while creating data");
+		}).finally(() => {
+			augBrowser.end();
+		});
+	});
+});

Review Comment:
   Yes, originally I needed browser (which isn't created when `before` is called), but since I don't need that I can def move it there.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 merged pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
ocket8888 merged PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805


-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r864946687


##########
experimental/traffic-portal/nightwatch/globals/globals.ts:
##########
@@ -21,11 +53,97 @@ export interface GlobalConfig extends NightwatchGlobals {
 	adminPass: string;
 	adminUser: string;
 	trafficOpsURL: string;
+	apiVersion: string;
 }
-const config = {
+const globals = {
 	adminPass: "twelve12",
 	adminUser: "admin",
+	apiVersion: "4.0",
+	before: async (done: () => void): Promise<void> => {
+		const apiUrl = `${globals.trafficOpsURL}/api/${globals.apiVersion}`;
+		const client = axios.create({
+			httpsAgent: new https.Agent({
+				rejectUnauthorized: false
+			})
+		});
+		let accessToken = "";
+		const loginReq: LoginRequest = {
+			p: globals.adminPass,
+			u: globals.adminUser
+		};
+		try {
+			const resp = await client.post(`${apiUrl}/user/login`, JSON.stringify(loginReq));
+			if(resp.headers["set-cookie"]) {
+				for (const cookie of resp.headers["set-cookie"]) {
+					if(cookie.indexOf("access_token") > -1) {
+						accessToken = cookie;
+						break;
+					}
+				}
+			}
+		} catch (e) {
+			console.error((e as AxiosError).message);
+			return Promise.reject();

Review Comment:
   This is equivalent to `throw undefined`, which you'd never do. In fact, if you just don't catch this at all, the Promise will reject with the thrown error which will bubble up the call stack until something handles it or it crashes the tests, in which case it gets logged for you.



##########
experimental/traffic-portal/nightwatch/tests/servers/servers.spec.ts:
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.
+ */
+
+/*
+* 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 {AugmentedBrowser} from "nightwatch/globals/globals";
+
+describe("Servers Spec", () => {
+	let augBrowser: AugmentedBrowser;
+	before(() => {
+		augBrowser = browser as AugmentedBrowser;
+	});
+
+	beforeEach(() => {
+		augBrowser.page.login()
+			.navigate().section.loginForm
+			.loginAndWait(augBrowser.globals.adminUser, augBrowser.globals.adminPass);
+	});
+
+	it("Filter by hostname", async () => {
+		const page = augBrowser.page.servers();
+		page.navigate()
+			.pause(4000)

Review Comment:
   I know this was in here already because I put it in, but is this actually necessary? Even if it is, I think what I should've done instead was add 4 seconds to the element wait timeout.



##########
experimental/traffic-portal/nightwatch/tests/ds/ds.card.ts:
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 {type AugmentedBrowser} from "nightwatch/globals/globals";
+
+describe("DS Card Spec", () => {
+	let augBrowser: AugmentedBrowser;
+	before(() => {
+		augBrowser = browser as AugmentedBrowser;
+	});
+
+	beforeEach(() => {
+		augBrowser.page.login()
+			.navigate().section.loginForm
+			.loginAndWait(augBrowser.globals.adminUser, augBrowser.globals.adminPass);
+	});

Review Comment:
   Do you need to re-authenticate before *each* test?



##########
experimental/traffic-portal/nightwatch/globals/globals.ts:
##########
@@ -21,11 +53,97 @@ export interface GlobalConfig extends NightwatchGlobals {
 	adminPass: string;
 	adminUser: string;
 	trafficOpsURL: string;
+	apiVersion: string;
 }
-const config = {
+const globals = {
 	adminPass: "twelve12",
 	adminUser: "admin",
+	apiVersion: "4.0",
+	before: async (done: () => void): Promise<void> => {
+		const apiUrl = `${globals.trafficOpsURL}/api/${globals.apiVersion}`;
+		const client = axios.create({
+			httpsAgent: new https.Agent({
+				rejectUnauthorized: false
+			})
+		});
+		let accessToken = "";
+		const loginReq: LoginRequest = {
+			p: globals.adminPass,
+			u: globals.adminUser
+		};
+		try {
+			const resp = await client.post(`${apiUrl}/user/login`, JSON.stringify(loginReq));
+			if(resp.headers["set-cookie"]) {
+				for (const cookie of resp.headers["set-cookie"]) {
+					if(cookie.indexOf("access_token") > -1) {
+						accessToken = cookie;
+						break;
+					}
+				}
+			}
+		} catch (e) {
+			console.error((e as AxiosError).message);
+			return Promise.reject();
+		}
+		if(accessToken === "") {
+			console.error("Access token is not set");
+			return Promise.reject();
+		}
+		client.defaults.headers.common = { Cookie: accessToken };
+
+		const cdn: CDN = {
+			dnssecEnabled: false, domainName: "tests.com", name: "testCDN"
+		};
+		let respCDN: ResponseCDN;
+		try {
+			let resp = await client.post(`${apiUrl}/cdns`, JSON.stringify(cdn));
+			respCDN = resp.data.response;
+
+			const ds: RequestDeliveryService = {
+				active: false,
+				cacheurl: null,
+				cdnId: respCDN.id,
+				displayName: "test DS",
+				dscp: 0,
+				ecsEnabled: false,
+				edgeHeaderRewrite: null,
+				fqPacingRate: null,
+				geoLimit: GeoLimit.NONE,
+				geoProvider: GeoProvider.MAX_MIND,
+				httpBypassFqdn: null,
+				infoUrl: null,
+				initialDispersion: 1,
+				ipv6RoutingEnabled: false,
+				logsEnabled: false,
+				maxOriginConnections: 0,
+				maxRequestHeaderBytes: 0,
+				midHeaderRewrite: null,
+				missLat: 0,
+				missLong: 0,
+				multiSiteOrigin: false,
+				orgServerFqdn: "http://test.com",
+				profileId: 1,
+				protocol: Protocol.HTTP,
+				qstringIgnore: 0,
+				rangeRequestHandling: 0,
+				regionalGeoBlocking: false,
+				remapText: null,
+				routingName: "test",
+				signed: false,
+				tenantId: 1,
+				typeId: 1,
+				xmlId: "testDS"
+			};
+			resp = await client.post(`${apiUrl}/deliveryservices`, JSON.stringify(ds));
+			const respDS: ResponseDeliveryService = resp.data.response[0];
+			console.log(`Successfully created DS '${respDS.displayName}'`);
+		} catch(e) {
+			console.error((e as AxiosError).message);
+			return Promise.reject();

Review Comment:
   same here



##########
experimental/traffic-portal/nightwatch/page_objects/deliveryServiceCard.ts:
##########
@@ -0,0 +1,77 @@
+/*
+* 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 {
+	EnhancedElementInstance,
+	EnhancedPageObject, EnhancedSectionInstance, NightwatchAPI
+} from "nightwatch";
+
+/**
+ * Defines the commands for the loginForm section
+ */
+interface DeliveryServiceCardCommands extends EnhancedSectionInstance, EnhancedElementInstance<EnhancedPageObject> {
+	expandDS(xmlId: string): Promise<boolean>;
+	viewDetails(xmlId: string): Promise<boolean>;
+}
+
+/**
+ * Defines the loginForm section
+ */
+type DeliveryServiceCardSection = EnhancedSectionInstance<DeliveryServiceCardCommands,
+	typeof deliveryServiceCardPageObject.sections.cards.elements>;
+
+/**
+ * Define the type for our PO
+ */
+export type DeliveryServiceCardPageObject = EnhancedPageObject<{}, {}, { cards: DeliveryServiceCardSection }>;
+
+const deliveryServiceCardPageObject = {
+	api: {} as NightwatchAPI,
+	sections: {
+		cards: {
+			commands: {
+				async expandDS(xmlId: string): Promise<boolean> {
+					return new Promise((resolve, reject) => {
+						this.click("css selector", `mat-card#${xmlId}`, result => {
+							if (result.status === 1) {
+								reject(new Error(`Unable to find by css mat-card#${xmlId}`));
+								return;
+							}
+							this.waitForElementVisible(`mat-card#${xmlId} mat-card-content > div`,
+								undefined, undefined, undefined, () => {
+									resolve(true);
+								});
+						});
+					});
+				},
+				async viewDetails(xmlId: string): Promise<boolean> {
+					await this.expandDS(xmlId);
+					return new Promise((resolve) => {
+						this.click("css selector", `mat-card#${xmlId} mat-card-actions > a`, () => {
+							browser.assert.urlContains("deliveryservice");

Review Comment:
   Does the Promise reject if this assertion fails? If it doesn't the caller might be unaware the navigation failed.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #6805: Add Various E2E TPv2 DS Tests

Posted by GitBox <gi...@apache.org>.
shamrickus commented on code in PR #6805:
URL: https://github.com/apache/trafficcontrol/pull/6805#discussion_r867254631


##########
experimental/traffic-portal/nightwatch/globals/globals.ts:
##########
@@ -21,11 +53,97 @@ export interface GlobalConfig extends NightwatchGlobals {
 	adminPass: string;
 	adminUser: string;
 	trafficOpsURL: string;
+	apiVersion: string;
 }
-const config = {
+const globals = {
 	adminPass: "twelve12",
 	adminUser: "admin",
+	apiVersion: "4.0",
+	before: async (done: () => void): Promise<void> => {
+		const apiUrl = `${globals.trafficOpsURL}/api/${globals.apiVersion}`;
+		const client = axios.create({
+			httpsAgent: new https.Agent({
+				rejectUnauthorized: false
+			})
+		});
+		let accessToken = "";
+		const loginReq: LoginRequest = {
+			p: globals.adminPass,
+			u: globals.adminUser
+		};
+		try {
+			const resp = await client.post(`${apiUrl}/user/login`, JSON.stringify(loginReq));
+			if(resp.headers["set-cookie"]) {
+				for (const cookie of resp.headers["set-cookie"]) {
+					if(cookie.indexOf("access_token") > -1) {
+						accessToken = cookie;
+						break;
+					}
+				}
+			}
+		} catch (e) {
+			console.error((e as AxiosError).message);
+			return Promise.reject();

Review Comment:
   That is how I had it originally; however, without the manual log, _nothing_ is output and the process exits. I can just change it to rethrow.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org