CodeHealth: Migrate PRESUBMITs from Python 2 to Python 3
This CL includes:
* set Pylint version to 2.7
* pass skip_shebang_check=True to RunUnitTestsInDirectory
And fixes some Pylint warnings:
* W0406: fix import-self
* E1101: Set name and aliases as an abstract property in
encode_pdf_filter.py since name and aliases are introduced in
subclasses.
* E0012: Remove Py2 hint of # pylint: disable=relative-import
* R0205: Replace all "class ClassName(objects)" with "class ClassName"
* E1120: Ignore this error. see
https://github.com/PyCQA/pylint/issues/3563
* W0707: explicitly re-raising using the 'from' keyword
Bug: pdfium:1674, chromium:1262354, chromium:1262297
Change-Id: I205d254514f1ae667af818c789b5e6d89a061c68
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/87270
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 55d417c..fa7241f 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -496,6 +496,9 @@
input_api,
output_api,
full_path,
- files_to_check=[r'^PRESUBMIT_test\.py$']))
+ files_to_check=[r'^PRESUBMIT_test\.py$'],
+ run_on_python2=not USE_PYTHON3,
+ run_on_python3=USE_PYTHON3,
+ skip_shebang_check=True))
return results
diff --git a/testing/tools/PRESUBMIT.py b/testing/tools/PRESUBMIT.py
index 02c256b..0a65565 100644
--- a/testing/tools/PRESUBMIT.py
+++ b/testing/tools/PRESUBMIT.py
@@ -13,7 +13,8 @@
def _CommonChecks(input_api, output_api):
tests = []
- tests.extend(input_api.canned_checks.GetPylint(input_api, output_api))
+ tests.extend(
+ input_api.canned_checks.GetPylint(input_api, output_api, version='2.7'))
return tests
diff --git a/testing/tools/api_check.py b/testing/tools/api_check.py
index f340243..5b582b5 100755
--- a/testing/tools/api_check.py
+++ b/testing/tools/api_check.py
@@ -101,7 +101,7 @@
def _FindDuplicates(functions):
- return set([f for f in functions if functions.count(f) > 1])
+ return {f for f in functions if functions.count(f) > 1}
def _CheckAndPrintFailures(failure_list, failure_message):
diff --git a/testing/tools/coverage/coverage_report.py b/testing/tools/coverage/coverage_report.py
index 06c03e4..32016fc 100755
--- a/testing/tools/coverage/coverage_report.py
+++ b/testing/tools/coverage/coverage_report.py
@@ -60,7 +60,7 @@
}
-class CoverageExecutor(object):
+class CoverageExecutor:
def __init__(self, parser, args):
"""Initialize executor based on the current script environment
@@ -276,7 +276,7 @@
print('Failed to successfully build binaries')
return False
- for name in self.coverage_tests.keys():
+ for name in self.coverage_tests:
if not self.generate_coverage(name, self.coverage_tests[name]):
print('Failed to successfully generate coverage data')
return False
diff --git a/testing/tools/encode_pdf_filter.py b/testing/tools/encode_pdf_filter.py
index 5f29c82..3e0be9c 100755
--- a/testing/tools/encode_pdf_filter.py
+++ b/testing/tools/encode_pdf_filter.py
@@ -13,6 +13,7 @@
"""
import argparse
+from abc import ABCMeta, abstractmethod
import base64
import collections
import collections.abc
@@ -21,10 +22,20 @@
import zlib
-class _PdfStream:
+class _PdfStream(metaclass=ABCMeta):
_unique_filter_classes = []
_filter_classes = {}
+ @property
+ @abstractmethod
+ def name(self):
+ pass
+
+ @property
+ @abstractmethod
+ def aliases(self):
+ pass
+
@staticmethod
def GetFilterByName(name):
# Tolerate any case-insensitive match for "/Name" or "Name", or an alias.
@@ -99,6 +110,20 @@
def __init__(self):
super().__init__(io.BytesIO())
+ @property
+ def name(self):
+ # Return an invalid name, so as to ensure _SinkPdfStream.Register()
+ # cannot be called. This method has to be implemented, because this
+ # script create `_SinkPdfStream` instances.
+ return ''
+
+ @property
+ def aliases(self):
+ # Return an invalid aliases, so as to ensure _SinkPdfStream.Register()
+ # cannot be called. This method has to be implemented, because this
+ # script create `_SinkPdfStream` instances.
+ return ()
+
def close(self):
# Don't call io.BytesIO.close(); this deallocates the written data.
self.flush()
@@ -114,6 +139,16 @@
self.wrapcol = wrapcol
self.column = 0
+ @property
+ @abstractmethod
+ def name(self):
+ pass
+
+ @property
+ @abstractmethod
+ def aliases(self):
+ pass
+
def write(self, data):
if not self.wrapcol:
self.buffer.write(data)
@@ -134,8 +169,16 @@
class _Ascii85DecodePdfStream(_AsciiPdfStream):
- name = '/ASCII85Decode'
- aliases = ('ascii85', 'base85')
+ _name = '/ASCII85Decode'
+ _aliases = ('ascii85', 'base85')
+
+ @property
+ def name(self):
+ return self._name
+
+ @property
+ def aliases(self):
+ return self._aliases
def __init__(self, out_buffer, **kwargs):
super().__init__(out_buffer, **kwargs)
@@ -158,8 +201,16 @@
class _AsciiHexDecodePdfStream(_AsciiPdfStream):
- name = '/ASCIIHexDecode'
- aliases = ('base16', 'hex', 'hexadecimal')
+ _name = '/ASCIIHexDecode'
+ _aliases = ('base16', 'hex', 'hexadecimal')
+
+ @property
+ def name(self):
+ return self._name
+
+ @property
+ def aliases(self):
+ return self._aliases
def __init__(self, out_buffer, **kwargs):
super().__init__(out_buffer, **kwargs)
@@ -169,13 +220,21 @@
class _FlateDecodePdfStream(_PdfStream):
- name = '/FlateDecode'
- aliases = ('deflate', 'flate', 'zlib')
+ _name = '/FlateDecode'
+ _aliases = ('deflate', 'flate', 'zlib')
def __init__(self, out_buffer, **kwargs):
super().__init__(out_buffer, **kwargs)
self.deflate = zlib.compressobj(level=9, memLevel=9)
+ @property
+ def name(cls):
+ return cls._name
+
+ @property
+ def aliases(self):
+ return self._aliases
+
def write(self, data):
self.buffer.write(self.deflate.compress(data))
@@ -189,6 +248,16 @@
class _VirtualPdfStream(_PdfStream):
+ @property
+ @abstractmethod
+ def name(self):
+ pass
+
+ @property
+ @abstractmethod
+ def aliases(self):
+ pass
+
@classmethod
def RegisterByName(cls):
pass
@@ -199,13 +268,21 @@
class _PassthroughPdfStream(_VirtualPdfStream):
- name = '(virtual) passthrough'
- aliases = ('noop', 'passthrough')
+ _name = '(virtual) passthrough'
+ _aliases = ('noop', 'passthrough')
+
+ @property
+ def name(self):
+ return self._name
+
+ @property
+ def aliases(self):
+ return self._aliases
class _PngIdatPdfStream(_VirtualPdfStream):
- name = '(virtual) PNG IDAT'
- aliases = ('png',)
+ _name = '(virtual) PNG IDAT'
+ _aliases = ('png',)
_EXPECT_HEADER = -1
_EXPECT_LENGTH = -2
@@ -215,6 +292,14 @@
_PNG_HEADER = 0x89504E470D0A1A0A
_PNG_CHUNK_IDAT = 0x49444154
+ @property
+ def name(self):
+ return self._name
+
+ @property
+ def aliases(self):
+ return self._aliases
+
@classmethod
def AddEntries(cls, entries):
# Technically only true for compression method 0 (zlib), but no other
diff --git a/testing/tools/githelper.py b/testing/tools/githelper.py
index 4637ed7..a8d62e7 100644
--- a/testing/tools/githelper.py
+++ b/testing/tools/githelper.py
@@ -5,11 +5,10 @@
import subprocess
-# pylint: disable=relative-import
from common import RunCommandPropagateErr
-class GitHelper(object):
+class GitHelper:
"""Issues git commands. Stateful."""
def __init__(self):
diff --git a/testing/tools/pngdiffer.py b/testing/tools/pngdiffer.py
index 911da10..1da4c46 100755
--- a/testing/tools/pngdiffer.py
+++ b/testing/tools/pngdiffer.py
@@ -11,7 +11,6 @@
import shutil
import sys
-# pylint: disable=relative-import
import common
@@ -42,7 +41,6 @@
class NotFoundError(Exception):
"""Raised when file doesn't exist"""
- pass
class PNGDiffer():
@@ -165,7 +163,8 @@
ACTUAL_TEMPLATE = '.pdf.%d.png'
-class PathTemplates(object):
+
+class PathTemplates:
def __init__(self, input_filename, source_dir, working_dir, os_name,
max_path_mode):
diff --git a/testing/tools/run_corpus_tests.py b/testing/tools/run_corpus_tests.py
index c81aea3..edc4f02 100755
--- a/testing/tools/run_corpus_tests.py
+++ b/testing/tools/run_corpus_tests.py
@@ -5,7 +5,6 @@
import sys
-# pylint: disable=relative-import
import test_runner
diff --git a/testing/tools/run_javascript_tests.py b/testing/tools/run_javascript_tests.py
index 948c1af..cb6e69a 100755
--- a/testing/tools/run_javascript_tests.py
+++ b/testing/tools/run_javascript_tests.py
@@ -5,7 +5,6 @@
import sys
-# pylint: disable=relative-import
import test_runner
diff --git a/testing/tools/run_pixel_tests.py b/testing/tools/run_pixel_tests.py
index 0992e0a..92523d1 100755
--- a/testing/tools/run_pixel_tests.py
+++ b/testing/tools/run_pixel_tests.py
@@ -5,7 +5,6 @@
import sys
-# pylint: disable=relative-import
import test_runner
diff --git a/testing/tools/safetynet_compare.py b/testing/tools/safetynet_compare.py
index 8a5942d..7ce7d5a 100755
--- a/testing/tools/safetynet_compare.py
+++ b/testing/tools/safetynet_compare.py
@@ -18,7 +18,6 @@
import sys
import tempfile
-# pylint: disable=relative-import
from common import GetBooleanGnArg
from common import PrintErr
from common import RunCommandPropagateErr
@@ -35,7 +34,7 @@
return (test_case, result)
-class CompareRun(object):
+class CompareRun:
"""A comparison between two branches of pdfium."""
def __init__(self, args):
@@ -529,8 +528,7 @@
output_filename = (
'callgrind.out.%s.%s' % (test_case.replace('/', '_'), run_label))
return os.path.join(self.args.output_dir, output_filename)
- else:
- return None
+ return None
def _DrawConclusions(self, times_before_branch, times_after_branch):
"""Draws conclusions comparing results of test runs in two branches.
diff --git a/testing/tools/safetynet_conclusions.py b/testing/tools/safetynet_conclusions.py
index dc6e3dd..fb563b5 100644
--- a/testing/tools/safetynet_conclusions.py
+++ b/testing/tools/safetynet_conclusions.py
@@ -33,7 +33,7 @@
}
-class ComparisonConclusions(object):
+class ComparisonConclusions:
"""All conclusions drawn from a comparison.
This is initialized empty and then processes pairs of results for each test
@@ -184,7 +184,7 @@
return output_dict
-class ComparisonSummary(object):
+class ComparisonSummary:
"""Totals computed for a comparison."""
def __init__(self):
@@ -209,7 +209,7 @@
return result
-class CaseResult(object):
+class CaseResult:
"""The conclusion for the comparison of a single test case."""
def __init__(self, case_name, before, after, ratio, rating):
diff --git a/testing/tools/safetynet_image.py b/testing/tools/safetynet_image.py
index 3161178..9232098 100644
--- a/testing/tools/safetynet_image.py
+++ b/testing/tools/safetynet_image.py
@@ -15,7 +15,6 @@
import sys
import webbrowser
-# pylint: disable=relative-import
from common import DirectoryFinder
@@ -23,7 +22,7 @@
return image_comparison.GenerateOneDiff(image)
-class ImageComparison(object):
+class ImageComparison:
"""Compares pairs of page images and generates an HTML to look at differences.
The images are all assumed to have the same name and be in two directories:
@@ -241,7 +240,7 @@
f.write('</td></tr>')
-class ImageLocations(object):
+class ImageLocations:
"""Contains the locations of input and output image files.
"""
diff --git a/testing/tools/safetynet_job.py b/testing/tools/safetynet_job.py
index c1361c1..ba1d055 100755
--- a/testing/tools/safetynet_job.py
+++ b/testing/tools/safetynet_job.py
@@ -15,14 +15,13 @@
import os
import sys
-# pylint: disable=relative-import
from common import PrintWithTime
from common import RunCommandPropagateErr
from githelper import GitHelper
from safetynet_conclusions import PrintConclusionsDictHumanReadable
-class JobContext(object):
+class JobContext:
"""Context for a single run, including name and directory paths."""
def __init__(self, args):
@@ -36,7 +35,7 @@
'%s.log' % self.datetime)
-class JobRun(object):
+class JobRun:
"""A single run looking for regressions since the last one."""
def __init__(self, args, context):
diff --git a/testing/tools/safetynet_measure.py b/testing/tools/safetynet_measure.py
index be61aa8..55738cb 100755
--- a/testing/tools/safetynet_measure.py
+++ b/testing/tools/safetynet_measure.py
@@ -15,7 +15,6 @@
import subprocess
import sys
-# pylint: disable=relative-import
from common import PrintErr
CALLGRIND_PROFILER = 'callgrind'
@@ -25,7 +24,7 @@
PDFIUM_TEST = 'pdfium_test'
-class PerformanceRun(object):
+class PerformanceRun:
"""A single measurement of a test case."""
def __init__(self, args):
diff --git a/testing/tools/skia_gold/__init__.py b/testing/tools/skia_gold/__init__.py
index a16cacc..1c98a89 100644
--- a/testing/tools/skia_gold/__init__.py
+++ b/testing/tools/skia_gold/__init__.py
@@ -1,3 +1,3 @@
-from . import path_util
+from .path_util import AddDirToPathIfNeeded, GetPDFiumDir
-path_util.AddDirToPathIfNeeded(path_util.GetPDFiumDir(), 'build')
+AddDirToPathIfNeeded(GetPDFiumDir(), 'build')
diff --git a/testing/tools/skia_gold/path_util.py b/testing/tools/skia_gold/path_util.py
index 94c9297..e9fb04a 100644
--- a/testing/tools/skia_gold/path_util.py
+++ b/testing/tools/skia_gold/path_util.py
@@ -8,6 +8,7 @@
def AddDirToPathIfNeeded(*path_parts):
+ # pylint: disable=no-value-for-parameter
path = os.path.abspath(os.path.join(*path_parts))
if os.path.isdir(path) and path not in sys.path:
sys.path.append(path)
diff --git a/testing/tools/skia_gold/pdfium_skia_gold_session.py b/testing/tools/skia_gold/pdfium_skia_gold_session.py
index 9f7d198..8b6d3d6 100644
--- a/testing/tools/skia_gold/pdfium_skia_gold_session.py
+++ b/testing/tools/skia_gold/pdfium_skia_gold_session.py
@@ -4,13 +4,12 @@
# found in the LICENSE file.
"""PDFium implementation of //build/skia_gold_common/skia_gold_session.py."""
-# pylint: disable=relative-import
from skia_gold_common import output_managerless_skia_gold_session as omsgs
# ComparisonResults nested inside the SkiaGoldSession causes issues with
# multiprocessing and pickling, so it was moved out here.
-class PDFiumComparisonResults(object):
+class PDFiumComparisonResults:
"""Struct-like object for storing results of an image comparison."""
def __init__(self):
diff --git a/testing/tools/skia_gold/skia_gold.py b/testing/tools/skia_gold/skia_gold.py
index 112a560..136beb1 100644
--- a/testing/tools/skia_gold/skia_gold.py
+++ b/testing/tools/skia_gold/skia_gold.py
@@ -97,7 +97,7 @@
os.makedirs(output_dir)
-class SkiaGoldTester(object):
+class SkiaGoldTester:
def __init__(self, source_type, skia_gold_args, process_name=None):
"""
@@ -175,7 +175,7 @@
self.GetSkiaGoldSessionManager().GetSessionClass().StatusCodes
if status == status_codes.SUCCESS:
return True
- elif status == status_codes.AUTH_FAILURE:
+ if status == status_codes.AUTH_FAILURE:
logging.error('Gold authentication failed with output %s', error)
elif status == status_codes.INIT_FAILURE:
logging.error('Gold initialization failed with output %s', error)
diff --git a/testing/tools/suppressor.py b/testing/tools/suppressor.py
index 9eec78b..c59941a 100755
--- a/testing/tools/suppressor.py
+++ b/testing/tools/suppressor.py
@@ -7,7 +7,6 @@
import os
-# pylint: disable=relative-import
import common
diff --git a/testing/tools/test_runner.py b/testing/tools/test_runner.py
index 3e935d9..f64d7a6 100644
--- a/testing/tools/test_runner.py
+++ b/testing/tools/test_runner.py
@@ -15,7 +15,6 @@
import subprocess
import sys
-# pylint: disable=relative-import
import common
import pngdiffer
import suppressor
@@ -49,6 +48,9 @@
result = this.GenerateAndTest(input_filename, source_dir)
return (result, input_filename, source_dir)
except KeyboardInterrupt:
+ # TODO(https://crbug.com/pdfium/1674): Re-enable this check after try bots
+ # can run with Python3.
+ # pylint: disable=raise-missing-from
raise KeyboardInterruptError()
@@ -64,6 +66,9 @@
results.append((test_name, skia_success, input_filename))
return results
except KeyboardInterrupt:
+ # TODO(https://crbug.com/pdfium/1674): Re-enable this check after try bots
+ # can run with Python3.
+ # pylint: disable=raise-missing-from
raise KeyboardInterruptError()
@@ -227,7 +232,7 @@
try:
with open(txt_path, "r") as txt_file:
txt_data = txt_file.readlines()
- if not len(txt_data):
+ if not txt_data:
return None
sys.stdout.write('Unexpected output:\n')
for line in txt_data:
