This is an automated email from the ASF dual-hosted git repository. bneradt pushed a commit to branch autopep8_only_on_tracked_files in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit dc4f6573a8fafb42a8c55c4436d158ee9f4e7344 Author: bneradt <[email protected]> AuthorDate: Sun Sep 13 03:00:21 2020 +0000 autopep8: avoid running on non-committed files. 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. --- 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 | 28 ++++++++++- tools/git/pre-commit | 2 + 11 files changed, 118 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 df8a497..23a30f1 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 94c8294..5544503 100644 --- a/tests/gold_tests/autest-site/microserver.test.ext +++ b/tests/gold_tests/autest-site/microserver.test.ext @@ -88,7 +88,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"] = [] @@ -172,7 +172,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) @@ -236,7 +248,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 0425ae0..6c7913e 100644 --- a/tests/gold_tests/autest-site/setup.cli.ext +++ b/tests/gold_tests/autest-site/setup.cli.ext @@ -46,7 +46,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 7b87c6c..3fe1020 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..02611b8 100755 --- a/tools/autopep8.sh +++ b/tools/autopep8.sh @@ -59,6 +59,28 @@ function main() { fi DIR=${@:-.} + + tmp_dir=/tmp/git_files_$$ + mkdir ${tmp_dir} + 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} + # 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 +89,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 \
