ocket8888 commented on code in PR #6696:
URL: https://github.com/apache/trafficcontrol/pull/6696#discussion_r845342314
##########
.github/actions/tpv2-integration-tests/package-lock.json:
##########
@@ -0,0 +1,1994 @@
+{
+ "name": "to-integration-tests",
+ "version": "0.0.0",
+ "lockfileVersion": 1,
Review Comment:
This is maddening. These tests use lockfile version 1, and so do the TO and
TPv1 integration tests, and the TP project itself. But TPv2 and the TPv1
end-to-end tests and the TPv1 front-end app all use lockfile version 2.
Lockfile version 3 already exists, and the `npm` bundled with the oldest LTS
Nodejs version still supported supports lockfile version 2. So I think we
should just use version 2 everywhere, because it causes thousands of lines of
diffs that don't actually reflect real changes every time we flip back and
forth between them.
##########
.github/actions/tpv2-integration-tests/tpv2-integration-tests.ts:
##########
@@ -0,0 +1,31 @@
+/*
+* 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.
+*/
+
+"use strict";
+import child_process, { SpawnSyncOptions, SpawnSyncReturns } from
"child_process";
Review Comment:
nit: This doesn't fail the linter settings (which btw we really need to
standardize so we don't have 30 eslint configs scattered around) but combining
object destructuring imports with default imports _looks_ weird, almost like it
shouldn't work. The types of `spawnOptions` and `output` are trivially
inferrable by their assignments, so if it were me I'd just not import those two
types, which also keeps the imports consistent throughout since `path` is using
the default import.
##########
experimental/traffic-portal/package.json:
##########
@@ -80,26 +82,30 @@
"@angular-eslint/template-parser": "13.1.0",
"@angular/cli": "^13.2.0",
"@angular/compiler-cli": "^13.2.0",
+ "@nightwatch/schematics": "^0.1.9",
Review Comment:
extra indent
##########
experimental/traffic-portal/nightwatch/globals/globals.ts:
##########
@@ -0,0 +1,35 @@
+/*
+*
+* 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 {NightwatchGlobals} from "nightwatch";
+
+/**
+ * Defines the configuration used for the testing environment
+ */
+export interface GlobalConfig extends NightwatchGlobals {
+ adminPass: string;
+ adminUser: string;
+ trafficOpsURL: string;
+}
+const config = {
+ chrome_headless: {},
+ default: {
+ adminPass: "twelve12",
+ adminUser: "admin",
+ trafficOpsURL: "https://localhost:6443";
+ }
+};
+config.chrome_headless = config.default;
Review Comment:
idk if this is actually any better, but you could do this inline in the
`config` literal as a `get` literal, fwiw. It's a little more convoluted, but
it makes the typing correct - the exported `config` has the type:
```typescript
{
chrome_headless: {},
default: {
adminPass: string,
adminUser: string,
trafficOpsUrl: string
}
}
```
because `typeof config.default` is assignable to `{}` the assignment on this
line succeeds, but anyone trying to access properties of `chrome_headless`
won't be able to because its type has no properties. Although that doesn't
appear to actually matter, for whatever reason.
##########
experimental/traffic-portal/angular.json:
##########
@@ -115,22 +117,11 @@
"options": {
"lintFilePatterns": [
"src/**/*.ts",
- "src/**/*.html"
+ "src/**/*.html",
+ "nightwatch/**/*.ts"
Review Comment:
This should also include `nightwatch/*.js` so that
`nightwatch/nightwatch.conf.js` is linted. It should really also do `*.ts` and
`src/*.ts`, but that's not a problem you need to deal with in this PR.
##########
experimental/traffic-portal/nightwatch/globals/globals.ts:
##########
@@ -0,0 +1,35 @@
+/*
+*
+* 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 {NightwatchGlobals} from "nightwatch";
Review Comment:
nit: could be `import type`
##########
.github/actions/tpv2-integration-tests/entrypoint.sh:
##########
@@ -0,0 +1,116 @@
+#!/bin/bash
+# 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.
+
+onFail() {
+ echo "Error on line ${1} of ${2}" >&2;
+ cd "${REPO_DIR}/experimental/traffic-portal"
+ if ! [[ -d Reports ]]; then
+ mkdir Reports;
+ fi
+ if [[ -d nightwatch/junit ]]; then
+ mv nightwatch/junit Reports
+ fi
+ if [[ -d nightwatch/screens ]]; then
+ mv nightwatch/screens Reports
+ fi
+ if [[ -d logs ]]; then
+ mv logs Reports
+ fi
+ if [[ -f "${REPO_DIR}/traffic_ops/traffic_ops_golang" ]]; then
+ cp "${REPO_DIR}/traffic_ops/traffic_ops_golang" Reports/to.log;
+ fi
+ echo "Detailed logs produced info Reports artifact"
+ exit 1
+}
+
+trap 'onFail "${LINENO}" "${0}"' ERR
+set -o errexit -o nounset -o pipefail
+
+to_fqdn="https://localhost:6443";
+tp_fqdn="http://localhost:4200";
+
+export PGUSER="traffic_ops"
+export PGPASSWORD="twelve"
+export PGHOST="localhost"
+export PGDATABASE="traffic_ops"
+export PGPORT="5432"
+
+to_admin_username="admin"
+to_admin_password="twelve12"
+password_hash="$(<<PYTHON_COMMANDS
PYTHONPATH="${GITHUB_WORKSPACE}/traffic_ops/install/bin" python
+import _postinstall
+print(_postinstall.hash_pass('${to_admin_password}'))
+PYTHON_COMMANDS
+)"
+<<QUERY psql
+INSERT INTO tm_user (username, role, tenant_id, local_passwd)
+ VALUES ('${to_admin_username}', 1, 1,
+ '${password_hash}'
+ );
+QUERY
+
+sudo useradd trafops
+
+ciab_dir="${GITHUB_WORKSPACE}/infrastructure/cdn-in-a-box";
+openssl rand 32 | base64 | sudo tee /aes.key
+
+sudo apt-get install -y --no-install-recommends gettext curl
+
+export GOPATH="${HOME}/go"
+readonly ORG_DIR="$GOPATH/src/github.com/apache"
+readonly REPO_DIR="${ORG_DIR}/trafficcontrol"
+resources="$(dirname "$0")"
+if [[ ! -e "$REPO_DIR" ]]; then
+ mkdir -p "$ORG_DIR"
+ cd
+ mv "${GITHUB_WORKSPACE}" "${REPO_DIR}/"
+ ln -s "$REPO_DIR" "${GITHUB_WORKSPACE}"
+fi
+
+to_build() {
+ pushd "${REPO_DIR}/traffic_ops/traffic_ops_golang"
+ if [[ ! -d "${GITHUB_WORKSPACE}/vendor/golang.org" ]]; then
+ go mod vendor
+ fi
+ go build .
+
+ openssl req -new -x509 -nodes -newkey rsa:4096 -out localhost.crt -keyout
localhost.key -subj "/CN=tptests";
+
+ envsubst <"${resources}/cdn.json" >cdn.conf
+ cp "${resources}/database.json" database.conf
+
+ truncate -s0 out.log
+ ./traffic_ops_golang --cfg ./cdn.conf --dbcfg ./database.conf >out.log 2>&1 &
+ popd
+}
+
+to_build
Review Comment:
I don't think there's much of a point to this function if it's called on the
very first line after its definition and it's only called once. It'd be clearer
and simpler to just run the commands it contains.
##########
experimental/traffic-portal/nightwatch/.eslintrc.json:
##########
@@ -0,0 +1,42 @@
+/*
+* 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.
+*/
+{
+ "extends": ["../.eslintrc.json"],
+ "overrides": [
+ {
+ "parserOptions": {
+ "project": [
+ "tsconfig.e2e.json"
+ ]
+ },
+ "files": [
+ "*.ts"
+ ],
+ "rules": {
+ "@typescript-eslint/naming-convention": [
+ "error",
+ {
+ "selector":
"objectLiteralProperty",
+ "format": ["camelCase"],
+ "filter": {
+ "regex":
"^[a-zA-Z0-9.,!?_ ]+$",
+ "match": false
+ }
+ }
+ ],
+
"@typescript-eslint/no-unnecessary-type-arguments": 0
Review Comment:
Is this override necessary?
##########
experimental/traffic-portal/nightwatch/.eslintrc.json:
##########
@@ -0,0 +1,42 @@
+/*
+* 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.
+*/
+{
+ "extends": ["../.eslintrc.json"],
+ "overrides": [
+ {
+ "parserOptions": {
+ "project": [
+ "tsconfig.e2e.json"
+ ]
+ },
+ "files": [
+ "*.ts"
Review Comment:
this array needs a `"*.js"` element to allow linting the
`nightwatch.conf.js` file.
##########
experimental/traffic-portal/.eslintrc.json:
##########
@@ -23,8 +23,7 @@
],
Review Comment:
In order for this config to be extended to `nightwatch/nightwatch.conf.js`,
the `files` property here needs to include an entry for `*.js`.
##########
experimental/traffic-portal/package.json:
##########
@@ -80,26 +82,30 @@
"@angular-eslint/template-parser": "13.1.0",
"@angular/cli": "^13.2.0",
"@angular/compiler-cli": "^13.2.0",
+ "@nightwatch/schematics": "^0.1.9",
"@types/argparse": "^2.0.2",
"@types/chart.js": "^2.9.34",
"@types/express": "^4.17.0",
"@types/jasmine": "~3.6.0",
"@types/jasminewd2": "~2.0.3",
+ "@types/nightwatch": "2.0.1",
"@types/node": "^14.17.34",
"@typescript-eslint/eslint-plugin": "^5.10.0",
"@typescript-eslint/parser": "^5.10.0",
+ "chromedriver": "^100.0.0",
"codelyzer": "^6.0.0",
"eslint": "^8.2.0",
"eslint-plugin-import": "^2.25.3",
"eslint-plugin-jsdoc": "^37.0.3",
"eslint-plugin-prefer-arrow": "^1.2.3",
+ "geckodriver": "3.0.1",
"jasmine-core": "^3.10.1",
"karma": "~6.3.2",
"karma-chrome-launcher": "~3.1.0",
"karma-coverage": "~2.2.0",
"karma-jasmine": "~4.0.0",
"karma-jasmine-html-reporter": "^1.5.0",
- "protractor": "~7.0.0",
+ "nightwatch": "2.0.0-beta.3",
Review Comment:
should we really be using a beta version?
##########
experimental/traffic-portal/nightwatch/tests/login.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.
+*/
+import {NightwatchBrowser} from "nightwatch";
+
+import {GlobalConfig} from "../globals/globals";
Review Comment:
nit: these two could also be `import type` imports
##########
experimental/traffic-portal/nightwatch/nightwatch.conf.js:
##########
@@ -0,0 +1,226 @@
+/*
+* 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.
+*/
+
+// Refer to the online docs for more details:
https://nightwatchjs.org/gettingstarted/configuration/
+const Services = {};
Review Comment:
Once ESLint _is_ parsing this file, it has a ton of problems. Most are
auto-fixable.
##########
experimental/traffic-portal/nightwatch/nightwatch.conf.js:
##########
@@ -0,0 +1,226 @@
+/*
+* 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.
+*/
+
+// Refer to the online docs for more details:
https://nightwatchjs.org/gettingstarted/configuration/
+const Services = {};
+loadServices();
+
+// _ _ _ _ _ _ _
+// | \ | |(_) | | | | | | | |
+// | \| | _ __ _ | |__ | |_ __ __ __ _ | |_ ___ | |__
+// | . ` || | / _` || '_ \ | __|\ \ /\ / / / _` || __| / __|| '_ \
+// | |\ || || (_| || | | || |_ \ V V / | (_| || |_ | (__ | | | |
+// \_| \_/|_| \__, ||_| |_| \__| \_/\_/ \__,_| \__| \___||_| |_|
+// __/ |
+// |___/
+
+module.exports = {
+ // An array of folders (excluding subfolders) where your tests are
located;
+ // if this is not specified, the test source must be passed as the
second argument to the test runner.
+ src_folders: ['./out-tsc/nightwatch/tests'],
+
+ // See https://nightwatchjs.org/guide/working-with-page-objects/
+ page_objects_path: './out-tsc/nightwatch/page_objects',
+
+ // See
https://nightwatchjs.org/guide/extending-nightwatch/#writing-custom-commands
+ custom_commands_path: '',
+
+ // See
https://nightwatchjs.org/guide/extending-nightwatch/#writing-custom-assertions
+ custom_assertions_path: '',
+
+ // See https://nightwatchjs.org/guide/#external-globals
+ globals_path: "../out-tsc/nightwatch/globals/globals.js",
+
+ webdriver: {},
+
+ test_settings: {
+ default: {
+ disable_error_log: false,
+ launch_url: 'http://localhost:4200',
+
+ screenshots: {
+ enabled: true,
+ path: 'nightwatch/screens',
+ on_failure: true
+ },
+
+ output_folder: "nightwatch/junit",
+
+ desiredCapabilities: {
+ browserName: 'chrome'
+ },
+
+ webdriver: {
+ start_process: true,
+ server_path: ''
+ }
+ },
+
+ safari: {
+ desiredCapabilities: {
+ browserName: 'safari',
+ alwaysMatch: {
+ acceptInsecureCerts: false
+ }
+ },
+ webdriver: {
+ start_process: true,
+ server_path: ''
+ }
+ },
+
+ firefox: {
+ desiredCapabilities: {
+ browserName: 'firefox',
+ alwaysMatch: {
+ acceptInsecureCerts: true,
+ 'moz:firefoxOptions': {
+ args: [
+ // '-headless',
+ // '-verbose'
+ ]
+ }
+ }
+ },
+ webdriver: {
+ start_process: true,
+ server_path: '',
+ cli_args: [
+ // very verbose geckodriver logs
+ // '-vv'
+ ]
+ }
+ },
+
+ chrome: {
+ desiredCapabilities: {
+ browserName: 'chrome',
+ 'goog:chromeOptions': {
+ // More info on Chromedriver:
https://sites.google.com/a/chromium.org/chromedriver/
+ //
+ // w3c:false tells Chromedriver to run
using the legacy JSONWire protocol (not required in Chrome 78)
+ w3c: true,
+ args: [
+ //'--no-sandbox',
+ //'--ignore-certificate-errors',
+ //'--allow-insecure-localhost',
+ //'--headless'
+ ]
+ }
+ },
+
+ webdriver: {
+ start_process: true,
+ server_path: '',
+ cli_args: [
+ // --verbose
+ ]
+ }
+ },
+
+ chrome_headless: {
+ extends: "chrome",
+
+ desiredCapabilities: {
+ 'goog:chromeOptions': {
+ args: [
+ '--headless'
+ ]
+ }
+ },
+ },
+
+ edge: {
+ desiredCapabilities: {
+ browserName: 'MicrosoftEdge',
+ 'ms:edgeOptions': {
+ w3c: true,
+ // More info on EdgeDriver:
https://docs.microsoft.com/en-us/microsoft-edge/webdriver-chromium/capabilities-edge-options
+ args: [
+ //'--headless'
+ ]
+ }
+ },
+
+ webdriver: {
+ start_process: true,
+ // Download msedgedriver from
https://docs.microsoft.com/en-us/microsoft-edge/webdriver-chromium/
+ // and set the location below:
+ server_path: '',
+ cli_args: [
+ // --verbose
+ ]
+ }
+ },
+
+
//////////////////////////////////////////////////////////////////////////////////
+ // Configuration for when using the Selenium service, either
locally or remote, |
+ // like Selenium Grid
|
+
//////////////////////////////////////////////////////////////////////////////////
+ selenium_server: {
+ // Selenium Server is running locally and is managed by
Nightwatch
+ selenium: {
+ start_process: true,
+ port: 4444,
+ cli_args: {
+ 'webdriver.gecko.driver':
(Services.geckodriver ? Services.geckodriver.path : ''),
+ 'webdriver.chrome.driver':
(Services.chromedriver ? Services.chromedriver.path : '')
+ }
+ }
+ },
+
+ 'selenium.chrome': {
+ extends: 'selenium_server',
+ desiredCapabilities: {
+ browserName: 'chrome',
+ chromeOptions: {
+ w3c: true,
+ args: [
+ "--headless"
+ ]
+ }
+ }
+ },
+
+ 'selenium.firefox': {
+ extends: 'selenium_server',
+ desiredCapabilities: {
+ browserName: 'firefox',
+ 'moz:firefoxOptions': {
+ args: [
+ // '-headless',
+ // '-verbose'
+ ]
+ }
+ }
+ }
+ }
+};
+
+function loadServices() {
+ try {
+ Services.seleniumServer = require('selenium-server');
Review Comment:
This will always fail because we don't require this dependency - should this
import be removed?
--
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]
