bug 1388313, fix warnings and deprecations, r=emin draft
authorAxel Hecht <axel@pike.org>
Thu, 17 Aug 2017 12:54:40 +0200
changeset 477 3e15728a75beda5b1a5b29a2919f147a321ae63c
parent 476 90d248e778ab14467f5411cb401caf0071279820
child 478 ac07dacaeeb7d19c2a946621e6306d3f04819e34
push id160
push useraxel@mozilla.com
push dateMon, 05 Mar 2018 14:24:27 +0000
reviewersemin
bugs1388313
bug 1388313, fix warnings and deprecations, r=emin This is quite a few open files, some TestCase.assert_, and unified newlines. The latter isn't really fixed, but I silence the warning. Right, python4 is going to remove that. MozReview-Commit-ID: 72irFWSsti
compare_locales/compare.py
compare_locales/parser.py
compare_locales/paths.py
compare_locales/tests/test_dtd.py
compare_locales/tests/test_merge.py
compare_locales/tests/test_merge_unknown.py
compare_locales/tests/test_util.py
--- a/compare_locales/compare.py
+++ b/compare_locales/compare.py
@@ -391,16 +391,18 @@ class ContentComparer:
             offset = 0
             for skip in skips:
                 chunk = skip.span
                 f.write(ctx.contents[offset:chunk[0]])
                 offset = chunk[1]
             f.write(ctx.contents[offset:])
 
         if not (capabilities & parser.CAN_MERGE):
+            if f:
+                f.close()
             return
 
         if skips or missing:
             if f is None:
                 self.create_merge_dir(merge_file)
                 shutil.copyfile(l10n_file.fullpath, merge_file)
                 f = codecs.open(merge_file, 'ab', encoding)
 
--- a/compare_locales/parser.py
+++ b/compare_locales/parser.py
@@ -4,16 +4,17 @@
 
 from __future__ import absolute_import
 from __future__ import unicode_literals
 import re
 import bisect
 import codecs
 from collections import Counter
 import logging
+import warnings
 
 try:
     from html import unescape as html_unescape
 except ImportError:
     from HTMLParser import HTMLParser
     html_parser = HTMLParser()
     html_unescape = html_parser.unescape
 
@@ -240,22 +241,25 @@ class Parser(object):
 
     def __init__(self):
         if not hasattr(self, 'encoding'):
             self.encoding = 'utf-8'
         self.ctx = None
         self.last_comment = None
 
     def readFile(self, file):
-        with open(file, 'rbU') as f:
-            try:
-                self.readContents(f.read())
-            except UnicodeDecodeError as e:
-                (logging.getLogger('locales')
-                        .error("Can't read file: " + file + '; ' + str(e)))
+        # ignore universal newlines deprecation
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore")
+            with open(file, 'rbU') as f:
+                try:
+                    self.readContents(f.read())
+                except UnicodeDecodeError as e:
+                    (logging.getLogger('locales')
+                            .error("Can't read file: " + file + '; ' + str(e)))
 
     def readContents(self, contents):
         '''Read contents and create parsing context.
 
         contents are in native encoding, but with normalized line endings.
         '''
         (contents, length) = codecs.getdecoder(self.encoding)(contents)
         self.ctx = Parser.Context(contents)
--- a/compare_locales/paths.py
+++ b/compare_locales/paths.py
@@ -5,16 +5,17 @@
 from __future__ import absolute_import
 import os
 import re
 from six.moves.configparser import ConfigParser, NoSectionError, NoOptionError
 from collections import defaultdict
 import errno
 import itertools
 import logging
+import warnings
 from compare_locales import util, mozpath
 import pytoml as toml
 import six
 
 
 class Matcher(object):
     '''Path pattern matcher
     Supports path matching similar to mozpath.match(), but does
@@ -552,17 +553,18 @@ class L10nConfigParser(object):
         '''Get the test functions from this ConfigParser and all children.
 
         Only works with synchronous loads, used by compare-locales, which
         is local anyway.
         '''
         filter_path = mozpath.join(mozpath.dirname(self.inipath), 'filter.py')
         try:
             local = {}
-            exec(compile(open(filter_path).read(), filter_path, 'exec'), {}, local)
+            with open(filter_path) as f:
+                exec(compile(f.read(), filter_path, 'exec'), {}, local)
             if 'test' in local and callable(local['test']):
                 filters = [local['test']]
             else:
                 filters = []
         except BaseException:  # we really want to handle EVERYTHING here
             filters = []
 
         for c in self.children:
@@ -626,17 +628,18 @@ class L10nConfigParser(object):
         for t in self.dirsIter():
             yield t
         for child in self.children:
             for t in child.directories():
                 yield t
 
     def allLocales(self):
         """Return a list of all the locales of this project"""
-        return util.parseLocales(open(self.all_path).read())
+        with open(self.all_path) as f:
+            return util.parseLocales(f.read())
 
 
 class SourceTreeConfigParser(L10nConfigParser):
     '''Subclassing L10nConfigParser to work with just the repos
     checked out next to each other instead of intermingled like
     we do for real builds.
     '''
 
@@ -676,17 +679,21 @@ class File(object):
         self.fullpath = fullpath
         self.file = file
         self.module = module
         self.locale = locale
         pass
 
     def getContents(self):
         # open with universal line ending support and read
-        return open(self.fullpath, 'rU').read()
+        # ignore universal newlines deprecation
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore")
+            with open(self.fullpath, 'rbU') as f:
+                return f.read()
 
     @property
     def localpath(self):
         f = self.file
         if self.module:
             f = mozpath.join(self.module, f)
         return f
 
--- a/compare_locales/tests/test_dtd.py
+++ b/compare_locales/tests/test_dtd.py
@@ -84,31 +84,31 @@ class TestDTD(ParserTestMixin, unittest.
 
     def test_license_header(self):
         p = parser.getParser('foo.dtd')
         p.readContents(self.resource('triple-license.dtd'))
         entities = list(p.walk())
         self.assertIsInstance(entities[0], parser.Comment)
         self.assertIn('MPL', entities[0].all)
         e = entities[2]
-        self.assert_(isinstance(e, parser.Entity))
+        self.assertIsInstance(e, parser.Entity)
         self.assertEqual(e.key, 'foo')
         self.assertEqual(e.val, 'value')
         self.assertEqual(len(entities), 4)
         p.readContents(b'''\
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - License, v. 2.0. If a copy of the MPL was not distributed with this file,
    - You can obtain one at http://mozilla.org/MPL/2.0/.  -->
 <!ENTITY foo "value">
 ''')
         entities = list(p.walk())
-        self.assert_(isinstance(entities[0], parser.Comment))
+        self.assertIsInstance(entities[0], parser.Comment)
         self.assertIn('MPL', entities[0].all)
         e = entities[2]
-        self.assert_(isinstance(e, parser.Entity))
+        self.assertIsInstance(e, parser.Entity)
         self.assertEqual(e.key, 'foo')
         self.assertEqual(e.val, 'value')
         self.assertEqual(len(entities), 4)
 
     def testBOM(self):
         self._test(u'\ufeff<!ENTITY foo.label "stuff">',
                    (('foo.label', 'stuff'),))
 
--- a/compare_locales/tests/test_merge.py
+++ b/compare_locales/tests/test_merge.py
@@ -58,17 +58,17 @@ eff = lEff word
             {'summary':
                 {None: {
                     'changed': 3,
                     'changed_w': 5
                 }},
              'details': {}
              }
         )
-        self.assert_(not os.path.exists(mozpath.join(self.tmp, "merge",
+        self.assertFalse(os.path.exists(mozpath.join(self.tmp, "merge",
                                                      'l10n.properties')))
 
     def testMissing(self):
         self.assertTrue(os.path.isdir(self.tmp))
         self.reference("""foo = fooVal
 bar = barVal
 eff = effVal""")
         self.localized("""bar = lBar
@@ -229,18 +229,18 @@ class TestDTD(unittest.TestCase, Content
             {'summary':
                 {None: {
                     'changed': 3,
                     'changed_w': 3
                 }},
              'details': {}
              }
         )
-        self.assert_(
-            not os.path.exists(mozpath.join(self.tmp, "merge", 'l10n.dtd')))
+        self.assertFalse(
+            os.path.exists(mozpath.join(self.tmp, "merge", 'l10n.dtd')))
 
     def testMissing(self):
         self.assertTrue(os.path.isdir(self.tmp))
         self.reference("""<!ENTITY foo 'fooVal'>
 <!ENTITY bar 'barVal'>
 <!ENTITY eff 'effVal'>""")
         self.localized("""<!ENTITY bar 'lBar'>
 """)
@@ -366,21 +366,23 @@ class TestDTD(unittest.TestCase, Content
              })
 
 
 class TestFluent(unittest.TestCase):
     maxDiff = None  # we got big dictionaries to compare
 
     def reference(self, content):
         self.ref = os.path.join(self.tmp, "en-reference.ftl")
-        open(self.ref, "w").write(content)
+        with open(self.ref, "w") as f:
+            f.write(content)
 
     def localized(self, content):
         self.l10n = os.path.join(self.tmp, "l10n.ftl")
-        open(self.l10n, "w").write(content)
+        with open(self.l10n, "w") as f:
+            f.write(content)
 
     def setUp(self):
         self.tmp = mkdtemp()
         os.mkdir(os.path.join(self.tmp, "merge"))
         self.ref = self.l10n = None
 
     def tearDown(self):
         shutil.rmtree(self.tmp)
@@ -412,17 +414,17 @@ bar = lBar
                     'changed_w': 3
                 }},
              'details': {}
              }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(not os.path.exists(mergepath))
+        self.assertFalse(os.path.exists(mergepath))
 
     def testMissing(self):
         self.reference("""\
 foo = fooVal
 bar = barVal
 -baz = bazVal
 eff = effVal
 """)
@@ -452,17 +454,17 @@ eff = lEff
                         'missing_w': 2,
                     }
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(not os.path.exists(mergepath))
+        self.assertFalse(os.path.exists(mergepath))
 
     def testBroken(self):
         self.reference("""\
 foo = fooVal
 bar = barVal
 eff = effVal
 """)
         self.localized("""\
@@ -503,17 +505,17 @@ eff = lEff {
                         'errors': 3
                     }
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(os.path.exists(mergepath))
+        self.assertTrue(os.path.exists(mergepath))
 
         p = getParser(mergepath)
         p.readFile(mergepath)
         merged_entities, merged_map = p.parse()
         self.assertEqual([e.key for e in merged_entities], ["foo"])
         merged_foo = merged_entities[merged_map['foo']]
 
         # foo should be l10n
@@ -544,17 +546,17 @@ foo = Localized { bar }
                         'changed_w': 1
                     }
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(not os.path.exists(mergepath))
+        self.assertFalse(os.path.exists(mergepath))
 
     def testMismatchingReferences(self):
         self.reference("""\
 foo = Reference { bar }
 bar = Reference { baz }
 baz = Reference
 """)
         self.localized("""\
@@ -601,17 +603,17 @@ baz = Localized { qux }
                         'warnings': 4
                     }
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(not os.path.exists(mergepath))
+        self.assertFalse(os.path.exists(mergepath))
 
     def testMismatchingAttributes(self):
         self.reference("""
 foo = Foo
 bar = Bar
   .tender = Attribute value
 eff = Eff
 """)
@@ -646,17 +648,17 @@ eff = lEff
                 'summary': {
                     None: {'changed': 3, 'changed_w': 5, 'errors': 2}
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(os.path.exists(mergepath))
+        self.assertTrue(os.path.exists(mergepath))
 
         p = getParser(mergepath)
         p.readFile(mergepath)
         merged_entities, merged_map = p.parse()
         self.assertEqual([e.key for e in merged_entities], ["eff"])
         merged_eff = merged_entities[merged_map['eff']]
 
         # eff should be l10n
@@ -705,17 +707,17 @@ eff = lEff
                         'missing_w': 1,
                     }
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(not os.path.exists(mergepath))
+        self.assertFalse(os.path.exists(mergepath))
 
     def testMismatchingValues(self):
         self.reference("""
 foo = Foo
   .foottr = something
 bar
   .tender = Attribute value
 """)
@@ -748,17 +750,17 @@ bar = lBar
                 'summary': {
                     None: {'changed': 2, 'changed_w': 4, 'errors': 2}
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(os.path.exists(mergepath))
+        self.assertTrue(os.path.exists(mergepath))
 
         p = getParser(mergepath)
         p.readFile(mergepath)
         merged_entities, _ = p.parse()
         self.assertEqual([e.key for e in merged_entities], [])
 
     def testMissingGroupComment(self):
         self.reference("""\
@@ -786,17 +788,17 @@ bar = lBar
                         'changed_w': 2,
                     }
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(not os.path.exists(mergepath))
+        self.assertFalse(os.path.exists(mergepath))
 
     def testMissingAttachedComment(self):
         self.reference("""\
 foo = fooVal
 
 # Attached Comment
 bar = barVal
 """)
@@ -821,17 +823,17 @@ bar = barVal
                         'unchanged_w': 1,
                     }
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(not os.path.exists(mergepath))
+        self.assertFalse(os.path.exists(mergepath))
 
     def testObsoleteStandaloneComment(self):
         self.reference("""\
 foo = fooVal
 bar = barVal
 """)
         self.localized("""\
 foo = lFoo
@@ -855,17 +857,17 @@ bar = lBar
                         'changed_w': 2,
                     }
                 }
             }
         )
 
         # validate merge results
         mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
-        self.assert_(not os.path.exists(mergepath))
+        self.assertFalse(os.path.exists(mergepath))
 
     def test_duplicate(self):
         self.assertTrue(os.path.isdir(self.tmp))
         self.reference("""foo = fooVal
 bar = barVal
 eff = effVal
 foo = other val for foo""")
         self.localized("""foo = localized
--- a/compare_locales/tests/test_merge_unknown.py
+++ b/compare_locales/tests/test_merge_unknown.py
@@ -1,21 +1,22 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import unittest
+import six
 
 from compare_locales.merge import merge_channels, MergeNotSupportedError
 
 
 class TestMergeUnknown(unittest.TestCase):
     name = "foo.unknown"
 
     def test_not_supported_error(self):
         channels = (b"""
 foo = Foo 1
 """, b"""
 foo = Foo 2
 """)
         pattern = "Unsupported file format \(foo\.unknown\)\."
-        with self.assertRaisesRegexp(MergeNotSupportedError, pattern):
+        with six.assertRaisesRegex(self, MergeNotSupportedError, pattern):
             merge_channels(self.name, *channels)
--- a/compare_locales/tests/test_util.py
+++ b/compare_locales/tests/test_util.py
@@ -5,26 +5,26 @@
 from __future__ import absolute_import
 import unittest
 
 from compare_locales import util
 
 
 class ParseLocalesTest(unittest.TestCase):
     def test_empty(self):
-        self.assertEquals(util.parseLocales(''), [])
+        self.assertEqual(util.parseLocales(''), [])
 
     def test_all(self):
-        self.assertEquals(util.parseLocales('''af
+        self.assertEqual(util.parseLocales('''af
 de'''), ['af', 'de'])
 
     def test_shipped(self):
-        self.assertEquals(util.parseLocales('''af
+        self.assertEqual(util.parseLocales('''af
 ja win mac
 de'''), ['af', 'de', 'ja'])
 
     def test_sparse(self):
-        self.assertEquals(util.parseLocales('''
+        self.assertEqual(util.parseLocales('''
 af
 
 de
 
 '''), ['af', 'de'])