pylint all Python scripts under testing/ (#38268)
diff --git a/ci/lint.sh b/ci/lint.sh index c1c4ce8..05c6d94 100755 --- a/ci/lint.sh +++ b/ci/lint.sh
@@ -75,6 +75,6 @@ "ci/" \ "impeller/" \ "tools/gn" \ - "testing/run_tests.py" + "testing/" echo "$(date +%T) Linting complete"
diff --git a/testing/android_systrace_test.py b/testing/android_systrace_test.py index e6c039d..1de674b 100755 --- a/testing/android_systrace_test.py +++ b/testing/android_systrace_test.py
@@ -15,7 +15,7 @@ PERFETTO_SESSION_KEY = 'session1' PERFETTO_TRACE_FILE = '/data/misc/perfetto-traces/trace' -PERFETTO_CONFIG = ''' +PERFETTO_CONFIG = """ write_into_file: true file_write_period_ms: 1000000000 flush_period_ms: 1000 @@ -32,10 +32,10 @@ } } } -''' +""" -def InstallApk(apk_path, package_name, adb_path='adb'): +def install_apk(apk_path, package_name, adb_path='adb'): print('Installing APK') subprocess.check_output([adb_path, 'shell', 'am', 'force-stop', package_name]) # Allowed to fail if APK was never installed. @@ -44,7 +44,7 @@ subprocess.check_output([adb_path, 'install', apk_path]) -def StartPerfetto(package_name, adb_path='adb'): +def start_perfetto(package_name, adb_path='adb'): print('Starting trace') cmd = [ adb_path, 'shell', 'echo', "'" + PERFETTO_CONFIG % package_name + "'", @@ -55,7 +55,7 @@ subprocess.check_output(cmd, stderr=subprocess.STDOUT) -def LaunchPackage(package_name, activity_name, adb_path='adb'): +def launch_package(package_name, activity_name, adb_path='adb'): print('Scanning logcat') subprocess.check_output([adb_path, 'logcat', '-c'], stderr=subprocess.STDOUT) logcat = subprocess.Popen([adb_path, 'logcat'], @@ -77,7 +77,7 @@ break -def CollectAndValidateTrace(adb_path='adb'): +def collect_and_validate_trace(adb_path='adb'): print('Fetching trace') subprocess.check_output([ adb_path, 'shell', 'perfetto', '--attach', PERFETTO_SESSION_KEY, '--stop' @@ -140,10 +140,10 @@ args = parser.parse_args() - InstallApk(args.apk_path, args.package_name, args.adb_path) - StartPerfetto(args.package_name, args.adb_path) - LaunchPackage(args.package_name, args.activity_name, args.adb_path) - return CollectAndValidateTrace(args.adb_path) + install_apk(args.apk_path, args.package_name, args.adb_path) + start_perfetto(args.package_name, args.adb_path) + launch_package(args.package_name, args.activity_name, args.adb_path) + return collect_and_validate_trace(args.adb_path) if __name__ == '__main__':
diff --git a/testing/benchmark/displaylist_benchmark_parser.py b/testing/benchmark/displaylist_benchmark_parser.py index a8b49c6..dac0f44 100755 --- a/testing/benchmark/displaylist_benchmark_parser.py +++ b/testing/benchmark/displaylist_benchmark_parser.py
@@ -8,52 +8,52 @@ import csv import json import sys -import matplotlib.pyplot as plt -from matplotlib.backends.backend_pdf import PdfPages as pdfp +import matplotlib.pyplot as plt # pylint: disable=import-error +from matplotlib.backends.backend_pdf import PdfPages as pdfp # pylint: disable=import-error -class BenchmarkResult: +class BenchmarkResult: # pylint: disable=too-many-instance-attributes - def __init__(self, name, backend, timeUnit, drawCallCount): + def __init__(self, name, backend, time_unit, draw_call_count): self.name = name self.series = {} - self.seriesLabels = {} + self.series_labels = {} self.backend = backend - self.largeYValues = False - self.yLimit = 200 - self.timeUnit = timeUnit - self.drawCallCount = drawCallCount - self.optionalValues = {} + self.large_y_values = False + self.y_limit = 200 + self.time_unit = time_unit + self.draw_call_count = draw_call_count + self.optional_values = {} def __repr__(self): return 'Name: % s\nBackend: % s\nSeries: % s\nSeriesLabels: % s\n' % ( - self.name, self.backend, self.series, self.seriesLabels + self.name, self.backend, self.series, self.series_labels ) - def addDataPoint(self, family, x, y): + def add_data_point(self, family, xval, yval): if family not in self.series: self.series[family] = {'x': [], 'y': []} - self.series[family]['x'].append(x) - self.series[family]['y'].append(y) + self.series[family]['x'].append(xval) + self.series[family]['y'].append(yval) - if y > self.yLimit: - self.largeYValues = True + if yval > self.y_limit: + self.large_y_values = True - def addOptionalValue(self, name, x, y): - if name not in self.optionalValues: - self.optionalValues[name] = {} + def add_optional_value(self, name, xval, yval): + if name not in self.optional_values: + self.optional_values[name] = {} - self.optionalValues[name][x] = y + self.optional_values[name][xval] = yval - def setFamilyLabel(self, family, label): + def set_family_label(self, family, label): # I'm not keying the main series dict off the family label # just in case we get data where the two aren't a 1:1 mapping - if family in self.seriesLabels: - assert self.seriesLabels[family] == label + if family in self.series_labels: + assert self.series_labels[family] == label return - self.seriesLabels[family] = label + self.series_labels[family] = label def plot(self): figures = [] @@ -63,22 +63,22 @@ plt.plot( self.series[family]['x'], self.series[family]['y'], - label=self.seriesLabels[family] + label=self.series_labels[family] ) plt.xlabel('Benchmark Seed') - plt.ylabel('Time (' + self.timeUnit + ')') + plt.ylabel('Time (' + self.time_unit + ')') title = '' # Crop the Y axis so that we can see what's going on at the lower end - if self.largeYValues: - plt.ylim((0, self.yLimit)) + if self.large_y_values: + plt.ylim((0, self.y_limit)) title = self.name + ' ' + self.backend + ' (Cropped)' else: title = self.name + ' ' + self.backend - if self.drawCallCount != -1: - title += '\nDraw Call Count: ' + str(int(self.drawCallCount)) + if self.draw_call_count != -1: + title += '\nDraw Call Count: ' + str(int(self.draw_call_count)) plt.title(title) @@ -87,22 +87,22 @@ plt.legend(fontsize='xx-small') plt.plot() - if self.largeYValues: + if self.large_y_values: # Plot again but with the full Y axis visible figures.append(plt.figure(dpi=1200, frameon=False, figsize=(11, 8.5))) for family in self.series: plt.plot( self.series[family]['x'], self.series[family]['y'], - label=self.seriesLabels[family] + label=self.series_labels[family] ) plt.xlabel('Benchmark Seed') - plt.ylabel('Time (' + self.timeUnit + ')') + plt.ylabel('Time (' + self.time_unit + ')') title = self.name + ' ' + self.backend + ' (Complete)' - if self.drawCallCount != -1: - title += '\nDraw Call Count: ' + str(int(self.drawCallCount)) + if self.draw_call_count != -1: + title += '\nDraw Call Count: ' + str(int(self.draw_call_count)) plt.title(title) @@ -113,25 +113,25 @@ return figures - def writeCSV(self, writer): + def write_csv(self, writer): # For now assume that all our series have the same x values # this is true for now, but may differ in the future with benchmark changes x_values = [] y_values = [] for family in self.series: x_values = ['x'] + self.series[family]['x'] - y_values.append([self.seriesLabels[family]] + self.series[family]['y']) + y_values.append([self.series_labels[family]] + self.series[family]['y']) - for name in self.optionalValues: + for name in self.optional_values: column = [name] - for key in self.optionalValues[name]: - column.append(self.optionalValues[name][key]) + for key in self.optional_values[name]: + column.append(self.optional_values[name][key]) y_values.append(column) - writer.writerow([self.name, self.drawCallCount]) - for line in range(len(x_values)): + writer.writerow([self.name, self.draw_call_count]) + for line, _ in enumerate(x_values): row = [x_values[line]] - for series in range(len(y_values)): + for series, _ in enumerate(y_values): row.append(y_values[series][line]) writer.writerow(row) @@ -147,7 +147,7 @@ parser.add_argument( '-o', '--output-pdf', - dest='outputPDF', + dest='output_pdf', action='store', default='output.pdf', help='Filename to output the PDF of graphs to.' @@ -155,23 +155,23 @@ parser.add_argument( '-c', '--output-csv', - dest='outputCSV', + dest='output_csv', action='store', default='output.csv', help='Filename to output the CSV data to.' ) args = parser.parse_args() - jsonData = parseJSON(args.filename) - return processBenchmarkData(jsonData, args.outputPDF, args.outputCSV) + json_data = parse_json(args.filename) + return process_benchmark_data(json_data, args.output_pdf, args.output_csv) def error(message): print(message) - exit(1) + sys.exit(1) -def extractAttributesLabel(benchmarkResult): +def extrac_attributes_label(benchmark_result): # Possible attribute keys are: # AntiAliasing # HairlineStroke @@ -182,7 +182,7 @@ for attr in attributes: try: - if benchmarkResult[attr] != 0: + if benchmark_result[attr] != 0: label += attr + ', ' except KeyError: pass @@ -190,96 +190,97 @@ return label[:-2] -def processBenchmarkData(benchmarkJSON, outputPDF, outputCSV): - benchmarkResultsData = {} +def process_benchmark_data(benchmark_json, output_pdf, output_csv): + benchmark_results_data = {} - for benchmarkResult in benchmarkJSON: + for benchmark_result in benchmark_json: # Skip aggregate results - if 'aggregate_name' in benchmarkResult: + if 'aggregate_name' in benchmark_result: continue - benchmarkVariant = benchmarkResult['name'].split('/') + benchmark_variant = benchmark_result['name'].split('/') # The final split is always `real_time` and can be discarded - benchmarkVariant.remove('real_time') + benchmark_variant.remove('real_time') - splits = len(benchmarkVariant) + splits = len(benchmark_variant) # First split is always the benchmark function name - benchmarkName = benchmarkVariant[0] + benchmark_name = benchmark_variant[0] # The last split is always the seeded value into the benchmark - benchmarkSeededValue = benchmarkVariant[splits - 1] + benchmark_seeded_value = benchmark_variant[splits - 1] # The second last split is always the backend - benchmarkBackend = benchmarkVariant[splits - 2] + benchmark_backend = benchmark_variant[splits - 2] # Time taken (wall clock time) for benchmark to run - benchmarkRealTime = benchmarkResult['real_time'] - benchmarkUnit = benchmarkResult['time_unit'] + benchmark_real_time = benchmark_result['real_time'] + benchmark_unit = benchmark_result['time_unit'] - benchmarkFamilyIndex = benchmarkResult['family_index'] + benchmark_family_index = benchmark_result['family_index'] - benchmarkFamilyLabel = '' + benchmark_family_label = '' if splits > 3: for i in range(1, splits - 2): - benchmarkFamilyLabel += benchmarkVariant[i] + ', ' + benchmark_family_label += benchmark_variant[i] + ', ' - benchmarkFamilyAttributes = extractAttributesLabel(benchmarkResult) + benchmark_family_attributes = extrac_attributes_label(benchmark_result) - if benchmarkFamilyAttributes == '': - benchmarkFamilyLabel = benchmarkFamilyLabel[:-2] + if benchmark_family_attributes == '': + benchmark_family_label = benchmark_family_label[:-2] else: - benchmarkFamilyLabel = benchmarkFamilyLabel + benchmarkFamilyAttributes + benchmark_family_label = benchmark_family_label + benchmark_family_attributes - if 'DrawCallCount' in benchmarkResult: - benchmarkDrawCallCount = benchmarkResult['DrawCallCount'] + if 'DrawCallCount' in benchmark_result: + benchmark_draw_call_count = benchmark_result['DrawCallCount'] else: - benchmarkDrawCallCount = -1 + benchmark_draw_call_count = -1 optional_keys = [ 'DrawCallCount_Varies', 'VerbCount', 'PointCount', 'VertexCount', 'GlyphCount' ] - if benchmarkName not in benchmarkResultsData: - benchmarkResultsData[benchmarkName] = BenchmarkResult( - benchmarkName, benchmarkBackend, benchmarkUnit, benchmarkDrawCallCount + if benchmark_name not in benchmark_results_data: + benchmark_results_data[benchmark_name] = BenchmarkResult( + benchmark_name, benchmark_backend, benchmark_unit, + benchmark_draw_call_count ) for key in optional_keys: - if key in benchmarkResult: - benchmarkResultsData[benchmarkName].addOptionalValue( - key, benchmarkSeededValue, benchmarkResult[key] + if key in benchmark_result: + benchmark_results_data[benchmark_name].add_optional_value( + key, benchmark_seeded_value, benchmark_result[key] ) - benchmarkResultsData[benchmarkName].addDataPoint( - benchmarkFamilyIndex, benchmarkSeededValue, benchmarkRealTime + benchmark_results_data[benchmark_name].add_data_point( + benchmark_family_index, benchmark_seeded_value, benchmark_real_time ) - benchmarkResultsData[benchmarkName].setFamilyLabel( - benchmarkFamilyIndex, benchmarkFamilyLabel + benchmark_results_data[benchmark_name].set_family_label( + benchmark_family_index, benchmark_family_label ) - pp = pdfp(outputPDF) + pdf = pdfp(output_pdf) - csv_file = open(outputCSV, 'w') + csv_file = open(output_csv, 'w') csv_writer = csv.writer(csv_file) - for benchmark in benchmarkResultsData: - figures = benchmarkResultsData[benchmark].plot() + for benchmark in benchmark_results_data: + figures = benchmark_results_data[benchmark].plot() for fig in figures: - pp.savefig(fig) - benchmarkResultsData[benchmark].writeCSV(csv_writer) - pp.close() + pdf.savefig(fig) + benchmark_results_data[benchmark].write_csv(csv_writer) + pdf.close() -def parseJSON(filename): +def parse_json(filename): try: - jsonFile = open(filename, 'r') - except: + json_file = open(filename, 'r') + except: # pylint: disable=bare-except error('Unable to load file.') try: - jsonData = json.load(jsonFile) - except JSONDecodeError: + json_data = json.load(json_file) + except JSONDecodeError: # pylint: disable=undefined-variable error('Invalid JSON. Unable to parse.') - return jsonData['benchmarks'] + return json_data['benchmarks'] if __name__ == '__main__':
diff --git a/testing/run_tests.py b/testing/run_tests.py index b3213c8..9d4603d 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py
@@ -1089,7 +1089,7 @@ if 'impeller-vulkan' in types: build_name = args.variant try: - xvfb.StartVirtualX(build_name, build_dir) + xvfb.start_virtual_x(build_name, build_dir) vulkan_gtest_filter = parse_impeller_vulkan_filter() gtest_flags = shuffle_flags gtest_flags.append(vulkan_gtest_filter) @@ -1101,7 +1101,7 @@ coverage=args.coverage ) finally: - xvfb.StopVirtualX(build_name) + xvfb.stop_virtual_x(build_name) if 'dart' in types: dart_filter = args.dart_filter.split(',') if args.dart_filter else None
diff --git a/testing/xvfb.py b/testing/xvfb.py index fe09429..079ddea 100644 --- a/testing/xvfb.py +++ b/testing/xvfb.py
@@ -6,27 +6,26 @@ """ import os -import platform import signal import subprocess import tempfile import time -def _XvfbDisplayIndex(_child_build_name): +def xvfb_display_index(_child_build_name): return '9' -def _XvfbPidFilename(child_build_name): +def xvfb_pid_filename(child_build_name): """Returns the filename to the Xvfb pid file. This name is unique for each builder. This is used by the linux builders.""" return os.path.join( tempfile.gettempdir(), - 'xvfb-' + _XvfbDisplayIndex(child_build_name) + '.pid' + 'xvfb-' + xvfb_display_index(child_build_name) + '.pid' ) -def StartVirtualX(child_build_name, build_dir): +def start_virtual_x(child_build_name, build_dir): """Start a virtual X server and set the DISPLAY environment variable so sub processes will use the virtual X server. Also start openbox. This only works on Linux and assumes that xvfb and openbox are installed. @@ -40,13 +39,13 @@ """ # We use a pid file to make sure we don't have any xvfb processes running # from a previous test run. - StopVirtualX(child_build_name) + stop_virtual_x(child_build_name) xdisplaycheck_path = None if build_dir: xdisplaycheck_path = os.path.join(build_dir, 'xdisplaycheck') - display = ':%s' % _XvfbDisplayIndex(child_build_name) + display = ':%s' % xvfb_display_index(child_build_name) # Note we don't add the optional screen here (+ '.0') os.environ['DISPLAY'] = display @@ -73,13 +72,13 @@ print('xdisplaycheck says there is a display still running, exiting...') raise Exception('Display already present.') - xvfb_lock_filename = '/tmp/.X%s-lock' % _XvfbDisplayIndex(child_build_name) + xvfb_lock_filename = '/tmp/.X%s-lock' % xvfb_display_index(child_build_name) if os.path.exists(xvfb_lock_filename): print('Removing stale xvfb lock file %r' % xvfb_lock_filename) try: os.unlink(xvfb_lock_filename) - except OSError as e: - print('Removing xvfb lock file failed: %s' % e) + except OSError as err: + print('Removing xvfb lock file failed: %s' % err) # Figure out which X server to try. cmd = 'Xvfb' @@ -93,8 +92,8 @@ stdout=subprocess.PIPE, stderr=subprocess.STDOUT, env=env) - xvfb_pid_filename = _XvfbPidFilename(child_build_name) - open(xvfb_pid_filename, 'w').write(str(proc.pid)) + pid_filename = xvfb_pid_filename(child_build_name) + open(pid_filename, 'w').write(str(proc.pid)) # Wait for Xvfb to start up. time.sleep(10) @@ -112,24 +111,24 @@ if xdisplayproc.returncode != 0: print('xdisplaycheck failed after %d seconds.' % checktime) print('xdisplaycheck output:') - for l in logs.splitlines(): - print('> %s' % l) - rc = proc.poll() - if rc is None: + for line in logs.splitlines(): + print('> %s' % line) + return_code = proc.poll() + if return_code is None: print('Xvfb still running, stopping.') proc.terminate() else: - print('Xvfb exited, code %d' % rc) + print('Xvfb exited, code %d' % return_code) print('Xvfb output:') - for l in proc.communicate()[0].splitlines(): - print('> %s' % l) + for line in proc.communicate()[0].splitlines(): + print('> %s' % line) raise Exception(logs) - else: - print('xdisplaycheck succeeded after %d seconds.' % checktime) - print('xdisplaycheck output:') - for l in logs.splitlines(): - print('> %s' % l) + + print('xdisplaycheck succeeded after %d seconds.' % checktime) + print('xdisplaycheck output:') + for line in logs.splitlines(): + print('> %s' % line) print('...OK') # Some ChromeOS tests need a window manager. @@ -137,18 +136,18 @@ print('Window manager (openbox) started.') -def StopVirtualX(child_build_name): +def stop_virtual_x(child_build_name): """Try and stop the virtual X server if one was started with StartVirtualX. When the X server dies, it takes down the window manager with it. If a virtual x server is not running, this method does nothing.""" - xvfb_pid_filename = _XvfbPidFilename(child_build_name) - if os.path.exists(xvfb_pid_filename): - xvfb_pid = int(open(xvfb_pid_filename).read()) + pid_filename = xvfb_pid_filename(child_build_name) + if os.path.exists(pid_filename): + xvfb_pid = int(open(pid_filename).read()) print('Stopping Xvfb with pid %d ...' % xvfb_pid) # If the process doesn't exist, we raise an exception that we can ignore. try: os.kill(xvfb_pid, signal.SIGKILL) except OSError: print('... killing failed, presuming unnecessary.') - os.remove(xvfb_pid_filename) + os.remove(pid_filename) print('Xvfb pid file removed')