You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2020/09/28 19:53:57 UTC
[trafficserver] branch 9.0.x updated: autopep8: avoid running on
non-tracked files. (#7186)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 9daae97 autopep8: avoid running on non-tracked files. (#7186)
9daae97 is described below
commit 9daae97b7c7854f08f6a736f925642853e28946b
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Mon Sep 14 11:37:52 2020 -0500
autopep8: avoid running on non-tracked files. (#7186)
It was found that if someone has non-commited Python files, such as can
happen if they have a virtual environment in their source tree, autopep8
will inspect those as well. This is slow and probably not desired by the
user.
This patch only runs autopep8 on files tracked by git. It will also
enable autopep8 to check .test.ext extensions, thus the updates to those
files.
It also has autopep8 run silently so it doesn't produce as much noise.
(cherry picked from commit a8150161bd2fbce3b67995e0ad8d65c2bc7e00ff)
---
tests/gold_tests/autest-site/cli_tools.test.ext | 13 ++---
tests/gold_tests/autest-site/conditions.test.ext | 19 +++++---
tests/gold_tests/autest-site/curl_header.test.ext | 56 +++++++++++-----------
tests/gold_tests/autest-site/init.cli.ext | 2 +-
tests/gold_tests/autest-site/ip.test.ext | 3 ++
tests/gold_tests/autest-site/microserver.test.ext | 25 ++++++++--
tests/gold_tests/autest-site/setup.cli.ext | 2 +-
.../gold_tests/autest-site/traffic_replay.test.ext | 8 +++-
.../autest-site/trafficserver_plugins.test.ext | 20 ++++----
tools/autopep8.sh | 31 +++++++++++-
tools/git/pre-commit | 2 +
11 files changed, 121 insertions(+), 60 deletions(-)
diff --git a/tests/gold_tests/autest-site/cli_tools.test.ext b/tests/gold_tests/autest-site/cli_tools.test.ext
index d82a30b..b253f03 100644
--- a/tests/gold_tests/autest-site/cli_tools.test.ext
+++ b/tests/gold_tests/autest-site/cli_tools.test.ext
@@ -31,19 +31,19 @@ Tools to help with TestRun commands
#
# Note that by default, the Default Process is created in this function.
-def spawn_commands(self, cmdstr, count, retcode = 0, use_default=True):
- ret=[]
+def spawn_commands(self, cmdstr, count, retcode=0, use_default=True):
+ ret = []
if use_default:
count = int(count) - 1
- for cnt in range(0,count):
+ for cnt in range(0, count):
ret.append(
self.Processes.Process(
name="cmdline-{num}".format(num=cnt),
- cmdstr = cmdstr,
- returncode = retcode
- )
+ cmdstr=cmdstr,
+ returncode=retcode
)
+ )
if use_default:
self.Processes.Default.Command = cmdstr
self.Processes.Default.ReturnCode = retcode
@@ -52,4 +52,5 @@ def spawn_commands(self, cmdstr, count, retcode = 0, use_default=True):
)
return ret
+
ExtendTestRun(spawn_commands, name="SpawnCommands")
diff --git a/tests/gold_tests/autest-site/conditions.test.ext b/tests/gold_tests/autest-site/conditions.test.ext
index a8a91c6..c9d3a0d 100644
--- a/tests/gold_tests/autest-site/conditions.test.ext
+++ b/tests/gold_tests/autest-site/conditions.test.ext
@@ -20,9 +20,10 @@ import subprocess
import json
import re
+
def HasOpenSSLVersion(self, version):
output = subprocess.check_output(
- os.path.join(self.Variables.BINDIR, "traffic_layout") + " info --versions --json", shell = True
+ os.path.join(self.Variables.BINDIR, "traffic_layout") + " info --versions --json", shell=True
)
json_data = output.decode('utf-8')
openssl_str = json.loads(json_data)['openssl_str']
@@ -34,8 +35,10 @@ def HasOpenSSLVersion(self, version):
"openssl library version is " + exe_ver + ", must be at least " + version
)
+
def HasCurlVersion(self, version):
- return self.EnsureVersion(["curl","--version"],min_version=version)
+ return self.EnsureVersion(["curl", "--version"], min_version=version)
+
def HasCurlFeature(self, feature):
@@ -58,6 +61,8 @@ def HasCurlFeature(self, feature):
default,
"Curl needs to support feature: {feature}".format(feature=feature)
)
+
+
def HasCurlOption(self, option):
def default(output):
tag = option.lower()
@@ -75,20 +80,23 @@ def HasCurlOption(self, option):
"Curl needs to support option: {option}".format(option=option)
)
+
def HasATSFeature(self, feature):
val = self.Variables.get(feature, None)
return self.Condition(
- lambda: val == True,
+ lambda: val,
"ATS feature not enabled: {feature}".format(feature=feature)
)
-#test if a plugin exists in the libexec folder
+# test if a plugin exists in the libexec folder
+
+
def PluginExists(self, pluginname):
path = os.path.join(self.Variables.PLUGINDIR, pluginname)
- return self.Condition(lambda: os.path.isfile(path) == True, path + " not found." )
+ return self.Condition(lambda: os.path.isfile(path), path + " not found.")
ExtendCondition(HasOpenSSLVersion)
@@ -97,4 +105,3 @@ ExtendCondition(HasCurlVersion)
ExtendCondition(HasCurlFeature)
ExtendCondition(HasCurlOption)
ExtendCondition(PluginExists)
-
diff --git a/tests/gold_tests/autest-site/curl_header.test.ext b/tests/gold_tests/autest-site/curl_header.test.ext
index e0ba992..3ce674e 100644
--- a/tests/gold_tests/autest-site/curl_header.test.ext
+++ b/tests/gold_tests/autest-site/curl_header.test.ext
@@ -22,6 +22,7 @@ import hosts.output as host
import re
+
class CurlHeader(Tester):
def __init__(self,
value,
@@ -38,13 +39,13 @@ class CurlHeader(Tester):
stack=host.getCurrentStack(1)
)
- ops = ("equal","equal_re")
+ ops = ("equal", "equal_re")
gold_dict = value
headers = tuple(gold_dict.keys())
if description is None:
- description = 'Checking that all {} headers exist and have matching values: {}'.format(len(headers), \
- ', '.join(headers) if len(headers) <= 10 else ', '.join(headers[0:10]) + ', etc.')
+ description = 'Checking that all {} headers exist and have matching values: {}'.format(
+ len(headers), ', '.join(headers) if len(headers) <= 10 else ', '.join(headers[0:10]) + ', etc.')
# sanity check for input dictionary format
for header, target in gold_dict.items():
@@ -54,34 +55,30 @@ class CurlHeader(Tester):
stack=self._stack
)
- if target == None or isinstance(target, str):
+ if target is None or isinstance(target, str):
continue
elif isinstance(target, dict):
for op, pos_val in target.items():
if op not in ops:
host.WriteError(
- 'CurlHeader Input: Unsupported operation \'{}\' for value at header \'{}\'. The available operations are: {}.'.format(op, header, ', '.join(ops)),
- stack=self._stack
- )
- elif pos_val == None or isinstance(pos_val, str):
+ 'CurlHeader Input: Unsupported operation \'{}\' for value at header \'{}\'. The available operations are: {}.'.format(
+ op, header, ', '.join(ops)), stack=self._stack)
+ elif pos_val is None or isinstance(pos_val, str):
continue
elif isinstance(pos_val, list):
for str_ in pos_val:
- if not isinstance(str_, str) and str_ != None:
+ if not isinstance(str_, str) and str_ is not None:
host.WriteError(
- 'CurlHeader Input: Value {} has unsupported type \'{}\' for header \'{}\'. Need to provide a string or None.'.format(str_, str_.__class__.__name__, header),
- stack=self._stack
- )
+ 'CurlHeader Input: Value {} has unsupported type \'{}\' for header \'{}\'. Need to provide a string or None.'.format(
+ str_, str_.__class__.__name__, header), stack=self._stack)
else:
host.WriteError(
- 'CurlHeader Input: Value {} has unsupported type \'{}\' for header \'{}\'. Need to provide a string, a list or None for possible curl values.'.format(pos_val, pos_val.__class__.__name__, header),
- stack=self._stack
- )
+ 'CurlHeader Input: Value {} has unsupported type \'{}\' for header \'{}\'. Need to provide a string, a list or None for possible curl values.'.format(
+ pos_val, pos_val.__class__.__name__, header), stack=self._stack)
else:
host.WriteError(
- 'CurlHeader Input: Value {} has unsupported type \'{}\' for header \'{}\'. Need to provide either a string, a dictionary or None.'.format(target, target.__class__.__name__, header),
- stack=self._stack
- )
+ 'CurlHeader Input: Value {} has unsupported type \'{}\' for header \'{}\'. Need to provide either a string, a dictionary or None.'.format(
+ target, target.__class__.__name__, header), stack=self._stack)
super(CurlHeader, self).__init__(
value=value,
@@ -120,16 +117,16 @@ class CurlHeader(Tester):
val_dict[vals[0].lower()] = ': '.join(vals[1:])
# generate a gold dictionary with lowercase header
- gold_dict = {k.lower() : v for k, v in gold_dict.items()}
+ gold_dict = {k.lower(): v for k, v in gold_dict.items()}
p_flag = 1
reason = ''
for header in gold_dict.keys():
v = val_dict.get(header)
- if v != None:
+ if v is not None:
res = self.match(gold_dict, header, v)
- if res == None:
+ if res is None:
continue
else:
reason += 'In field: {} \n'.format(header) + res
@@ -165,7 +162,7 @@ class CurlHeader(Tester):
def match(self, gold_dictionary, header, val_value):
target = gold_dictionary[header]
- if target == None:
+ if target is None:
return None
elif isinstance(target, str):
# if given a string, check for an exact match
@@ -175,21 +172,21 @@ class CurlHeader(Tester):
elif isinstance(target, dict):
# if given a dict, check for valid operations indicated by the keys
for op, pos_val in target.items():
- if pos_val == None:
+ if pos_val is None:
return None
elif isinstance(pos_val, list):
# print('Need to provide a list of possible values.')
# continue
if op == 'equal':
for str_ in pos_val:
- if val_value == str_ or str_ == None:
+ if val_value == str_ or str_ is None:
return None
# return 'No matching strings in the list.'
elif op == 'equal_re':
for regex in pos_val:
- if regex == None:
+ if regex is None:
return None
- elif re.fullmatch(regex, val_value) != None:
+ elif re.fullmatch(regex, val_value) is not None:
return None
elif isinstance(pos_val, str):
@@ -199,12 +196,12 @@ class CurlHeader(Tester):
# else 'Not an exact match.\nExpected : {}\nActual : {}'.format(pos_val, val_value)
elif op == 'equal_re':
- if re.fullmatch(pos_val, val_value) != None:
+ if re.fullmatch(pos_val, val_value) is not None:
return None
ret = ''
- ops = {'equal' : 'Any of the following strings: ',
- 'equal_re' : 'Any of the following regular expression: '}
+ ops = {'equal': 'Any of the following strings: ',
+ 'equal_re': 'Any of the following regular expression: '}
for op, pos_val in target.items():
ret += ' {}: \'{}\'\n'.format(ops[op], '\', \''.join(pos_val)) if isinstance(pos_val, list) \
@@ -212,4 +209,5 @@ class CurlHeader(Tester):
return 'Value \'{}\' matches none of: \n{}'.format(val_value, ret.strip('\n'))
+
AddTester(CurlHeader)
diff --git a/tests/gold_tests/autest-site/init.cli.ext b/tests/gold_tests/autest-site/init.cli.ext
index 37ffa96..a8d0641 100644
--- a/tests/gold_tests/autest-site/init.cli.ext
+++ b/tests/gold_tests/autest-site/init.cli.ext
@@ -26,7 +26,7 @@ autest_version = "1.8.1"
if AuTestVersion() < autest_version:
host.WriteError(
"Tests need AuTest version {needed_version} or better, found version {found_version}\n"
- "Please update AuTest:\n pip install --upgrade autest\n".format(
+ "Please update AuTest:\n pip install --upgrade autest\n".format(
needed_version=autest_version,
found_version=AuTestVersion()),
show_stack=False)
diff --git a/tests/gold_tests/autest-site/ip.test.ext b/tests/gold_tests/autest-site/ip.test.ext
index cc951cf..74ea68f 100755
--- a/tests/gold_tests/autest-site/ip.test.ext
+++ b/tests/gold_tests/autest-site/ip.test.ext
@@ -27,11 +27,14 @@ from ports import get_port
# For each argument, a port will be reserved, and its number will be assigned to the new variable for the
# argument.
#
+
+
def get_tcp_port(obj, *newVariables):
for v in newVariables:
if not isinstance(v, str):
raise TypeError("all function arguments must be strings")
get_port(obj, v)
+
#AddTestEntityMember(get_tcp_port, name="GetTcpPort")
ExtendTest(get_tcp_port, name="GetTcpPort")
diff --git a/tests/gold_tests/autest-site/microserver.test.ext b/tests/gold_tests/autest-site/microserver.test.ext
index b579136..5be699b 100644
--- a/tests/gold_tests/autest-site/microserver.test.ext
+++ b/tests/gold_tests/autest-site/microserver.test.ext
@@ -81,7 +81,7 @@ def addTransactionToSession(txn, JFile):
# hard coding only 1 session per file
# since for the purpose of testing, we don't need multiple sessions in a file
- if jsondata == None:
+ if jsondata is None:
jsondata = {}
jsondata["sessions"] = []
@@ -165,7 +165,19 @@ def uServerUpAndRunning(serverHost, port, isSsl, isIPv6, request, clientcert='',
AddWhenFunction(uServerUpAndRunning)
-def MakeOriginServer(obj, name, port=None, s_port=None, ip='INADDR_LOOPBACK', delay=None, ssl=False, lookup_key=DEFAULT_LOOKUP_KEY, clientcert='', clientkey='', both=False, options={}):
+def MakeOriginServer(
+ obj,
+ name,
+ port=None,
+ s_port=None,
+ ip='INADDR_LOOPBACK',
+ delay=None,
+ ssl=False,
+ lookup_key=DEFAULT_LOOKUP_KEY,
+ clientcert='',
+ clientkey='',
+ both=False,
+ options={}):
data_dir = os.path.join(obj.RunDirectory, name)
p = obj.Processes.Process(name)
@@ -229,7 +241,14 @@ def MakeOriginServer(obj, name, port=None, s_port=None, ip='INADDR_LOOPBACK', de
"options": {"skipHooks": None}
})
- p.Ready = When.uServerUpAndRunning(ipaddr, s_port if ssl else port, ssl, IPConstants.isIPv6(ip), healthcheck_request["headers"], clientcert=clientcert, clientkey=clientkey)
+ p.Ready = When.uServerUpAndRunning(
+ ipaddr,
+ s_port if ssl else port,
+ ssl,
+ IPConstants.isIPv6(ip),
+ healthcheck_request["headers"],
+ clientcert=clientcert,
+ clientkey=clientkey)
p.ReturnCode = Any(None, 0)
return p
diff --git a/tests/gold_tests/autest-site/setup.cli.ext b/tests/gold_tests/autest-site/setup.cli.ext
index 6045f31..c28c5d4 100644
--- a/tests/gold_tests/autest-site/setup.cli.ext
+++ b/tests/gold_tests/autest-site/setup.cli.ext
@@ -36,7 +36,7 @@ if ENV['ATS_BIN'] is not None:
hint = ''
if os.path.isfile(os.path.join(ENV['ATS_BIN'], 'bin', 'traffic_layout')):
hint = "\nDid you mean '--ats-bin {}'?".\
- format(os.path.join(ENV['ATS_BIN'],'bin'))
+ format(os.path.join(ENV['ATS_BIN'], 'bin'))
host.WriteError("traffic_layout is not found. Aborting tests - Bad build or install.{}".format(hint), show_stack=False)
try:
out = subprocess.check_output([traffic_layout, "--json"])
diff --git a/tests/gold_tests/autest-site/traffic_replay.test.ext b/tests/gold_tests/autest-site/traffic_replay.test.ext
index 0779dd5..c4b2e69 100644
--- a/tests/gold_tests/autest-site/traffic_replay.test.ext
+++ b/tests/gold_tests/autest-site/traffic_replay.test.ext
@@ -17,9 +17,11 @@
# limitations under the License.
# default 'mixed' for connection type since it doesn't hurt
+
+
def Replay(obj, name, replay_dir, key=None, cert=None, conn_type='mixed', options={}):
# ATS setup - one line because we leave records and remap config to user
- ts = obj.MakeATSProcess("ts", select_ports=False) # select ports can be disabled once we add ssl port selection in extension
+ ts = obj.MakeATSProcess("ts", select_ports=False) # select ports can be disabled once we add ssl port selection in extension
# TEMP
ts.Variables.ssl_port = 4443
@@ -62,7 +64,8 @@ def Replay(obj, name, replay_dir, key=None, cert=None, conn_type='mixed', option
if not cert:
cert = os.path.join(obj.Variables["AtsTestToolsDir"], "microserver", "ssl", "server.crt")
- command = 'traffic-replay --log_dir {0} --type {1} --verify --host {2} --port {3} --s_port {4} '.format(data_dir, conn_type, hostIP, ts.Variables.port, ts.Variables.ssl_port)
+ command = 'traffic-replay --log_dir {0} --type {1} --verify --host {2} --port {3} --s_port {4} '.format(
+ data_dir, conn_type, hostIP, ts.Variables.port, ts.Variables.ssl_port)
if key:
command += "-k {0} ".format(key)
@@ -88,4 +91,5 @@ def Replay(obj, name, replay_dir, key=None, cert=None, conn_type='mixed', option
# return all the stuff in case user wants to do extra optimization
return (ts, server, dns, tr)
+
AddTestRunSet(Replay)
diff --git a/tests/gold_tests/autest-site/trafficserver_plugins.test.ext b/tests/gold_tests/autest-site/trafficserver_plugins.test.ext
index a5d8177..4e234bc 100644
--- a/tests/gold_tests/autest-site/trafficserver_plugins.test.ext
+++ b/tests/gold_tests/autest-site/trafficserver_plugins.test.ext
@@ -47,21 +47,21 @@ def prepare_plugin_helper(so_name, tsproc, plugin_args="", copy_plugin=True):
plugin_dir = tsproc.Env['PROXY_CONFIG_PLUGIN_PLUGIN_DIR']
if copy_plugin:
host.WriteVerbose(
- "prepare_plugin",
- "Copying down {} into {}.".format(so_name, plugin_dir))
+ "prepare_plugin",
+ "Copying down {} into {}.".format(so_name, plugin_dir))
tsproc.Setup.Copy(so_name, plugin_dir)
else:
host.WriteVerbose(
- "prepare_plugin",
- "Skipping copying {} into {} due to configuration.".format(
- so_name, plugin_dir))
+ "prepare_plugin",
+ "Skipping copying {} into {} due to configuration.".format(
+ so_name, plugin_dir))
# Add an entry to plugin.config.
basename = os.path.basename(so_name)
config_line = "{0} {1}".format(basename, plugin_args)
host.WriteVerbose(
- "prepare_plugin",
- 'Adding line to plugin.config: "{}"'.format(config_line))
+ "prepare_plugin",
+ 'Adding line to plugin.config: "{}"'.format(config_line))
tsproc.Disk.plugin_config.AddLine(config_line)
@@ -80,7 +80,7 @@ def prepare_test_plugin(self, so_path, tsproc, plugin_args=""):
"""
if not os.path.exists(so_path):
raise ValueError(
- 'PrepareTestPlugin: file does not exist: "{}"'.format(so_path))
+ 'PrepareTestPlugin: file does not exist: "{}"'.format(so_path))
prepare_plugin_helper(so_path, tsproc, plugin_args, copy_plugin=True)
@@ -100,8 +100,8 @@ def prepare_installed_plugin(self, so_name, tsproc, plugin_args=""):
"""
if os.path.dirname(so_name):
raise ValueError(
- 'PrepareInstalledPlugin expects a filename not a path: '
- '"{}"'.format(so_name))
+ 'PrepareInstalledPlugin expects a filename not a path: '
+ '"{}"'.format(so_name))
prepare_plugin_helper(so_name, tsproc, plugin_args, copy_plugin=False)
diff --git a/tools/autopep8.sh b/tools/autopep8.sh
index 76b5a1b..b17e902 100755
--- a/tools/autopep8.sh
+++ b/tools/autopep8.sh
@@ -59,6 +59,31 @@ function main() {
fi
DIR=${@:-.}
+
+ # Only run autopep8 on tracked files. This saves time and possibly avoids
+ # formatting files the user doesn't want formatted.
+ tmp_dir=$(mktemp -d -t tracked-git-files.XXXXXXXXXX)
+ files=${tmp_dir}/git_files.txt
+ files_filtered=${tmp_dir}/git_files_filtered.txt
+ git ls-tree -r HEAD --name-only ${DIR} | grep -vE "lib/yamlcpp" > ${files}
+ # Add to the above any newly added staged files.
+ git diff --cached --name-only --diff-filter=A >> ${files}
+ # Keep this list of Python extensions the same with the list of
+ # extensions searched for in the tools/git/pre-commit hook.
+ grep -E '\.py$|\.cli.ext$|\.test.ext$' ${files} > ${files_filtered}
+
+ echo "Running autopep8. This may take a minute."
+ autopep8 \
+ --ignore-local-config \
+ -i \
+ -j 0 \
+ --exclude "${DIR}/lib/yamlcpp" \
+ --max-line-length 132 \
+ --aggressive \
+ --aggressive \
+ $(cat ${files_filtered})
+ # The above will not catch the Python files in the metalink tests because
+ # they do not have extensions.
autopep8 \
--ignore-local-config \
-i \
@@ -67,8 +92,10 @@ function main() {
--max-line-length 132 \
--aggressive \
--aggressive \
- --verbose \
- -r ${DIR}
+ --recursive \
+ plugins/experimental/metalink/test
+ echo "autopep8 completed."
+ rm -rf ${tmp_dir}
deactivate
}
diff --git a/tools/git/pre-commit b/tools/git/pre-commit
index e5c997f..1ece2cd 100755
--- a/tools/git/pre-commit
+++ b/tools/git/pre-commit
@@ -62,6 +62,8 @@ git diff-index --cached --diff-filter=ACMR --name-only HEAD | grep -vE "lib/yaml
*.cc | *.c | *.h | *.h.in)
${FORMAT} "$file" | diff -u "$file" - >> "$clang_patch_file"
;;
+ # Keep this list of Python extensions the same with the list of
+ # extensions searched for in the toosl/autopep8.sh script.
*.py | *.cli.ext | *.test.ext)
autopep8 \
--ignore-local-config \