servo: Merge #19325 - Report an errror if a package has duplicates allowed but there are no duplicates (from m-novikov:tidy-ignore-without-duplicates); r=jdm
authorMaxim Novikov <mnovikov.work@gmail.com>
Wed, 22 Nov 2017 11:49:40 -0600
changeset 437802 c5f7fb5d3028d87a8e5a15cb885e936daa48b76d
parent 437801 81abb6b13cbce2178bb614a5b829d9cf905a8edf
child 437803 268ab0070722577819bf296d18cc0d44ccb7f914
push id117
push userfmarier@mozilla.com
push dateTue, 28 Nov 2017 20:17:16 +0000
reviewersjdm
milestone59.0a1
servo: Merge #19325 - Report an errror if a package has duplicates allowed but there are no duplicates (from m-novikov:tidy-ignore-without-duplicates); r=jdm Resolves: #19306 <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #19306 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: a812af51d614bd96402da91d8f648ce51b301a1b
servo/python/tidy/servo_tidy/tidy.py
servo/python/tidy/servo_tidy_tests/test_tidy.py
--- a/servo/python/tidy/servo_tidy/tidy.py
+++ b/servo/python/tidy/servo_tidy/tidy.py
@@ -334,20 +334,27 @@ def check_lock(file_name, contents):
         if "replace" in package:
             continue
         source = package.get("source", "")
         if source == r"registry+https://github.com/rust-lang/crates.io-index":
             source = "crates.io"
         packages_by_name.setdefault(package["name"], []).append((package["version"], source))
 
     for (name, packages) in packages_by_name.iteritems():
-        if name in exceptions or len(packages) <= 1:
+        has_duplicates = len(packages) > 1
+        duplicates_allowed = name in exceptions
+
+        if has_duplicates == duplicates_allowed:
             continue
 
-        message = "duplicate versions for package `{}`".format(name)
+        if duplicates_allowed:
+            message = 'duplicates for `{}` are allowed, but only single version found'.format(name)
+        else:
+            message = "duplicate versions for package `{}`".format(name)
+
         packages.sort()
         packages_dependencies = list(find_reverse_dependencies(name, content))
         for version, source in packages:
             short_source = source.split("#")[0].replace("git+", "")
             message += "\n\t\033[93mThe following packages depend on version {} from '{}':\033[0m" \
                        .format(version, short_source)
             for name, dependency in packages_dependencies:
                 if version in dependency and short_source in dependency:
--- a/servo/python/tidy/servo_tidy_tests/test_tidy.py
+++ b/servo/python/tidy/servo_tidy_tests/test_tidy.py
@@ -228,16 +228,34 @@ class CheckTidiness(unittest.TestCase):
         msg2 = """duplicate versions for package `test3`
 \t\x1b[93mThe following packages depend on version 0.5.1 from 'crates.io':\x1b[0m
 \t\ttest4
 \t\x1b[93mThe following packages depend on version 0.5.1 from 'https://github.com/user/test3':\x1b[0m
 \t\ttest5"""
         self.assertEqual(msg2, errors.next()[2])
         self.assertNoMoreErrors(errors)
 
+    def test_lock_ignore_without_duplicates(self):
+        tidy.config["ignore"]["packages"] = ["test", "test2", "test3", "test5"]
+        errors = tidy.collect_errors_for_files(iterFile('duplicated_package.lock'), [tidy.check_lock], [], print_text=False)
+
+        msg = (
+            "duplicates for `test2` are allowed, but only single version found"
+            "\n\t\x1b[93mThe following packages depend on version 0.1.0 from 'https://github.com/user/test2':\x1b[0m"
+        )
+        self.assertEqual(msg, errors.next()[2])
+
+        msg2 = (
+            "duplicates for `test5` are allowed, but only single version found"
+            "\n\t\x1b[93mThe following packages depend on version 0.1.0 from 'https://github.com/':\x1b[0m"
+        )
+        self.assertEqual(msg2, errors.next()[2])
+
+        self.assertNoMoreErrors(errors)
+
     def test_lint_runner(self):
         test_path = base_path + 'lints/'
         runner = tidy.LintRunner(only_changed_files=False, progress=False)
         runner.path = test_path + 'some-fictional-file'
         self.assertEqual([(runner.path, 0, "file does not exist")], list(runner.check()))
         runner.path = test_path + 'not_script'
         self.assertEqual([(runner.path, 0, "lint should be a python script")],
                          list(runner.check()))