Skip to content

Commit

Permalink
checkdeps: encode input to os.walk as utf-8.
Browse files Browse the repository at this point in the history
Also fix checkdeps_test.py after the move into chromium/src.

Bug: 962059
Change-Id: I86f041e5ce819e0fbf3fb1e4de29fdde6c8d4596
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610222
Auto-Submit: John Budorick <jbudorick@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659173}
  • Loading branch information
jbudorick authored and Commit Bot committed May 13, 2019
1 parent 59bd5d3 commit fd4b2b6
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 55 deletions.
65 changes: 32 additions & 33 deletions buildtools/checkdeps/checkdeps_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ class CheckDepsTest(unittest.TestCase):
def setUp(self):
self.deps_checker = checkdeps.DepsChecker(
being_tested=True,
base_directory=os.path.join(os.path.dirname(__file__), os.path.pardir))
base_directory=os.path.join(os.path.dirname(__file__), '..', '..'))

def ImplTestRegularCheckDepsRun(self, ignore_temp_rules, skip_tests):
self.deps_checker._ignore_temp_rules = ignore_temp_rules
self.deps_checker._skip_tests = skip_tests
self.deps_checker.CheckDirectory(
os.path.join(self.deps_checker.base_directory,
'checkdeps/testdata'))
'buildtools/checkdeps/testdata'))

problems = self.deps_checker.results_formatter.GetResults()
if skip_tests:
Expand Down Expand Up @@ -56,13 +56,13 @@ def VerifySubstringsInProblems(key_path, substrings_in_sequence):

if ignore_temp_rules:
VerifySubstringsInProblems('testdata/allowed/test.h',
['-checkdeps/testdata/disallowed',
['-buildtools/checkdeps/testdata/disallowed',
'temporarily_allowed.h',
'-third_party/explicitly_disallowed',
'Because of no rule applying'])
else:
VerifySubstringsInProblems('testdata/allowed/test.h',
['-checkdeps/testdata/disallowed',
['-buildtools/checkdeps/testdata/disallowed',
'-third_party/explicitly_disallowed',
'Because of no rule applying'])

Expand All @@ -80,7 +80,7 @@ def VerifySubstringsInProblems(key_path, substrings_in_sequence):

if not skip_tests:
VerifySubstringsInProblems('allowed/not_a_test.cc',
['-checkdeps/testdata/disallowed'])
['-buildtools/checkdeps/testdata/disallowed'])

def testRegularCheckDepsRun(self):
self.ImplTestRegularCheckDepsRun(False, False)
Expand All @@ -99,7 +99,7 @@ def CountViolations(self, ignore_temp_rules):
self.deps_checker.results_formatter = results.CountViolationsFormatter()
self.deps_checker.CheckDirectory(
os.path.join(self.deps_checker.base_directory,
'checkdeps/testdata'))
'buildtools/checkdeps/testdata'))
return self.deps_checker.results_formatter.GetResults()

def testCountViolations(self):
Expand All @@ -111,17 +111,17 @@ def testCountViolationsIgnoringTempRules(self):
def testCountViolationsWithRelativePath(self):
self.deps_checker.results_formatter = results.CountViolationsFormatter()
self.deps_checker.CheckDirectory(
os.path.join('checkdeps', 'testdata', 'allowed'))
os.path.join('buildtools', 'checkdeps', 'testdata', 'allowed'))
self.failUnlessEqual('4', self.deps_checker.results_formatter.GetResults())

def testTempRulesGenerator(self):
self.deps_checker.results_formatter = results.TemporaryRulesFormatter()
self.deps_checker.CheckDirectory(
os.path.join(self.deps_checker.base_directory,
'checkdeps/testdata/allowed'))
'buildtools/checkdeps/testdata/allowed'))
temp_rules = self.deps_checker.results_formatter.GetResults()
expected = [u' "!checkdeps/testdata/disallowed/bad.h",',
u' "!checkdeps/testdata/disallowed/teststuff/bad.h",',
expected = [u' "!buildtools/checkdeps/testdata/disallowed/bad.h",',
u' "!buildtools/checkdeps/testdata/disallowed/teststuff/bad.h",',
u' "!third_party/explicitly_disallowed/bad.h",',
u' "!third_party/no_rule/bad.h",']
self.failUnlessEqual(expected, temp_rules)
Expand All @@ -134,36 +134,36 @@ def testBadBaseDirectoryNotCheckoutRoot(self):

def testCheckAddedIncludesAllGood(self):
problems = self.deps_checker.CheckAddedCppIncludes(
[['checkdeps/testdata/allowed/test.cc',
['#include "checkdeps/testdata/allowed/good.h"',
'#include "checkdeps/testdata/disallowed/allowed/good.h"']
[['buildtools/checkdeps/testdata/allowed/test.cc',
['#include "buildtools/checkdeps/testdata/allowed/good.h"',
'#include "buildtools/checkdeps/testdata/disallowed/allowed/good.h"']
]])
self.failIf(problems)

def testCheckAddedIncludesManyGarbageLines(self):
garbage_lines = ["My name is Sam%d\n" % num for num in range(50)]
problems = self.deps_checker.CheckAddedCppIncludes(
[['checkdeps/testdata/allowed/test.cc', garbage_lines]])
[['buildtools/checkdeps/testdata/allowed/test.cc', garbage_lines]])
self.failIf(problems)

def testCheckAddedIncludesNoRule(self):
problems = self.deps_checker.CheckAddedCppIncludes(
[['checkdeps/testdata/allowed/test.cc',
[['buildtools/checkdeps/testdata/allowed/test.cc',
['#include "no_rule_for_this/nogood.h"']
]])
self.failUnless(problems)

def testCheckAddedIncludesSkippedDirectory(self):
problems = self.deps_checker.CheckAddedCppIncludes(
[['checkdeps/testdata/disallowed/allowed/skipped/test.cc',
[['buildtools/checkdeps/testdata/disallowed/allowed/skipped/test.cc',
['#include "whatever/whocares.h"']
]])
self.failIf(problems)

def testCheckAddedIncludesTempAllowed(self):
problems = self.deps_checker.CheckAddedCppIncludes(
[['checkdeps/testdata/allowed/test.cc',
['#include "checkdeps/testdata/disallowed/temporarily_allowed.h"']
[['buildtools/checkdeps/testdata/allowed/test.cc',
['#include "buildtools/checkdeps/testdata/disallowed/temporarily_allowed.h"']
]])
self.failUnless(problems)

Expand All @@ -179,11 +179,11 @@ def testCopyIsDeep(self):
# once the bug is fixed, but succeed (with a temporary allowance)
# if the bug is in place.
problems = self.deps_checker.CheckAddedCppIncludes(
[['checkdeps/testdata/allowed/test.cc',
['#include "/checkdeps/testdata/disallowed/temporarily_allowed.h"']
[['buildtools/checkdeps/testdata/allowed/test.cc',
['#include "buildtools/checkdeps/testdata/disallowed/temporarily_allowed.h"']
],
['checkdeps/testdata/disallowed/foo_unittest.cc',
['#include "checkdeps/testdata/bongo/temp_allowed_for_tests.h"']
['buildtools/checkdeps/testdata/disallowed/foo_unittest.cc',
['#include "buildtools/checkdeps/testdata/bongo/temp_allowed_for_tests.h"']
]])
# With the bug in place, there would be two problems reported, and
# the second would be for foo_unittest.cc.
Expand All @@ -192,7 +192,7 @@ def testCopyIsDeep(self):

def testTraversalIsOrdered(self):
dirs_traversed = []
for rules, filenames in self.deps_checker.GetAllRulesAndFiles():
for rules, filenames in self.deps_checker.GetAllRulesAndFiles(dir_name='buildtools'):
self.failUnlessEqual(type(filenames), list)
self.failUnlessEqual(filenames, sorted(filenames))
if filenames:
Expand All @@ -203,38 +203,37 @@ def testTraversalIsOrdered(self):

def testCheckPartialImportsAreAllowed(self):
problems = self.deps_checker.CheckAddedProtoImports(
[['checkdeps/testdata/test.proto',
[['buildtools/checkdeps/testdata/test.proto',
['import "no_rule_for_this/nogood.proto"']
]])
self.failIf(problems)

def testCheckAddedFullPathImportsAllowed(self):
# NOTE: Base directory is buildtools.
problems = self.deps_checker.CheckAddedProtoImports(
[['checkdeps/testdata/test.proto',
['import "checkdeps/testdata/allowed/good.proto"',
'import "checkdeps/testdata/disallowed/sub_folder/good.proto"']
[['buildtools/checkdeps/testdata/test.proto',
['import "buildtools/checkdeps/testdata/allowed/good.proto"',
'import "buildtools/checkdeps/testdata/disallowed/sub_folder/good.proto"']
]])
self.failIf(problems)

def testCheckAddedFullPathImportsDisallowed(self):
problems = self.deps_checker.CheckAddedProtoImports(
[['checkdeps/testdata/test.proto',
['import "checkdeps/testdata/disallowed/bad.proto"']
[['buildtools/checkdeps/testdata/test.proto',
['import "buildtools/checkdeps/testdata/disallowed/bad.proto"']
]])
self.failUnless(problems)

def testCheckAddedFullPathImportsManyGarbageLines(self):
garbage_lines = ["My name is Sam%d\n" % num for num in range(50)]
problems = self.deps_checker.CheckAddedProtoImports(
[['checkdeps/testdata/test.proto',
[['buildtools/checkdeps/testdata/test.proto',
garbage_lines]])
self.failIf(problems)

def testCheckAddedIncludesNoRuleFullPath(self):
problems = self.deps_checker.CheckAddedProtoImports(
[['checkdeps/testdata/test.proto',
['import "../tools/some.proto"']
[['buildtools/checkdeps/testdata/test.proto',
['import "tools/some.proto"']
]])
self.failUnless(problems)

Expand Down
2 changes: 1 addition & 1 deletion buildtools/checkdeps/java_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def _IgnoreDir(self, d):
return False

def _PrescanFiles(self, added_classset):
for root, dirs, files in os.walk(self._base_directory):
for root, dirs, files in os.walk(self._base_directory.encode('utf-8')):
# Skip unwanted subdirectories. TODO(husky): it would be better to do
# this via the skip_child_includes flag in DEPS files. Maybe hoist this
# prescan logic into checkdeps.py itself?
Expand Down
4 changes: 2 additions & 2 deletions buildtools/checkdeps/testdata/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
include_rules = [
"-checkdeps/testdata/disallowed",
"+checkdeps/testdata/allowed",
"-buildtools/checkdeps/testdata/disallowed",
"+buildtools/checkdeps/testdata/allowed",
"-third_party/explicitly_disallowed",
]
skip_child_includes = [
Expand Down
8 changes: 4 additions & 4 deletions buildtools/checkdeps/testdata/allowed/DEPS
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
include_rules = [
"+checkdeps/testdata/disallowed/allowed",
"!checkdeps/testdata/disallowed/temporarily_allowed.h",
"+buildtools/checkdeps/testdata/disallowed/allowed",
"!buildtools/checkdeps/testdata/disallowed/temporarily_allowed.h",
"+third_party/allowed_may_use",
]

specific_include_rules = {
".*_unittest\.cc": [
"+checkdeps/testdata/disallowed/teststuff",
"!checkdeps/testdata/bongo/temp_allowed_for_tests.h",
"+buildtools/checkdeps/testdata/disallowed/teststuff",
"!buildtools/checkdeps/testdata/bongo/temp_allowed_for_tests.h",
]
}
2 changes: 1 addition & 1 deletion buildtools/checkdeps/testdata/allowed/foo_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "checkdeps/testdata/disallowed/teststuff/good.h"
#include "buildtools/checkdeps/testdata/disallowed/teststuff/good.h"
2 changes: 1 addition & 1 deletion buildtools/checkdeps/testdata/allowed/not_a_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "checkdeps/testdata/disallowed/teststuff/bad.h"
#include "buildtools/checkdeps/testdata/disallowed/teststuff/bad.h"
8 changes: 4 additions & 4 deletions buildtools/checkdeps/testdata/allowed/test.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "checkdeps/testdata/allowed/good.h"
#include "checkdeps/testdata/disallowed/bad.h"
#include "checkdeps/testdata/disallowed/allowed/good.h"
#include "checkdeps/testdata/disallowed/temporarily_allowed.h"
#include "buildtools/checkdeps/testdata/allowed/good.h"
#include "buildtools/checkdeps/testdata/disallowed/bad.h"
#include "buildtools/checkdeps/testdata/disallowed/allowed/good.h"
#include "buildtools/checkdeps/testdata/disallowed/temporarily_allowed.h"
#include "third_party/explicitly_disallowed/bad.h"
#include "third_party/allowed_may_use/good.h"
#include "third_party/no_rule/bad.h"
6 changes: 3 additions & 3 deletions buildtools/checkdeps/testdata/disallowed/allowed/test.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "checkdeps/testdata/allowed/good.h"
#include "buildtools/checkdeps/testdata/allowed/good.h"
// Always allowed to include self and parents.
#include "checkdeps/testdata/disallowed/good.h"
#include "checkdeps/testdata/disallowed/allowed/good.h"
#include "buildtools/checkdeps/testdata/disallowed/good.h"
#include "buildtools/checkdeps/testdata/disallowed/allowed/good.h"
#include "third_party/explicitly_disallowed/bad.h"
#include "third_party/allowed_may_use/bad.h"
#include "third_party/no_rule/bad.h"
2 changes: 1 addition & 1 deletion buildtools/checkdeps/testdata/disallowed/foo_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
// bug where we were taking shallow copies of rules when generating
// rules for subdirectories, so all rule objects were getting the same
// dictionary for specific rules.
#include "checkdeps/testdata/disallowed/temp_allowed_for_tests.h"
#include "buildtools/checkdeps/testdata/disallowed/temp_allowed_for_tests.h"
6 changes: 3 additions & 3 deletions buildtools/checkdeps/testdata/disallowed/test.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "checkdeps/testdata/allowed/good.h"
#include "buildtools/checkdeps/testdata/allowed/good.h"
// Always allowed to include self.
#include "checkdeps/testdata/disallowed/good.h"
#include "checkdeps/testdata/disallowed/allowed/good.h"
#include "buildtools/checkdeps/testdata/disallowed/good.h"
#include "buildtools/checkdeps/testdata/disallowed/allowed/good.h"
#include "third_party/explicitly_disallowed/bad.h"
// Only allowed for code under allowed/.
#include "third_party/allowed_may_use/bad.h"
Expand Down
4 changes: 2 additions & 2 deletions buildtools/checkdeps/testdata/noparent/test.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// found in the LICENSE file.

// Disallowed because noparent removes the +allowed from the parent dir.
#include "checkdeps/testdata/allowed/bad.h"
#include "buildtools/checkdeps/testdata/allowed/bad.h"

// Same-directory includes are still allowed.
#include "checkdeps/testdata/noparent/self.h"
#include "buildtools/checkdeps/testdata/noparent/self.h"

0 comments on commit fd4b2b6

Please sign in to comment.