diff --git a/.arclint b/.arclint index 84e032bb7..620591976 100644 --- a/.arclint +++ b/.arclint @@ -1,40 +1,44 @@ { "linters": { "generated": { "type": "generated" }, "clang-format": { "type": "clang-format", "version": "7.0", "bin": ["clang-format-7", "clang-format"], "include": "(^src/.*\\.(h|c|cpp)$)", "exclude": [ "(^src/(secp256k1|univalue|leveldb)/)" ] }, "autopep8": { "type": "autopep8", "include": "(\\.py$)" }, "flake8": { "type": "flake8", "include": "(\\.py$)", "flags": [ "--select=F401" ] }, "lint-format-strings": { "type": "script-and-regex", "include": "(^src/.*\\.(h|c|cpp)$)", "exclude": [ "(^src/(secp256k1|univalue|leveldb)/)" ], "script-and-regex.script": "test/lint/lint-format-strings.sh", "script-and-regex.regex": "/^(?P.+): (?P.+:.+)$/m" }, "check-doc": { "type": "check-doc", "include": "(^src/.*\\.(h|c|cpp)$)" + }, + "lint-tests": { + "type": "lint-tests", + "include": "(^src/(rpc/|wallet/)?test/.*\\.(cpp)$)" } } } diff --git a/arcanist/.phutil_module_cache b/arcanist/.phutil_module_cache index 0fa9952ef..755242af2 100644 --- a/arcanist/.phutil_module_cache +++ b/arcanist/.phutil_module_cache @@ -1 +1 @@ -{"__symbol_cache_version__":11,"21dded7bc13561b46b1b39ecbfaa6a3f":{"have":{"class":{"ClangFormatLinter":79}},"need":{"function":{"pht":302,"id":1342},"class":{"ArcanistExternalLinter":105,"ArcanistLintMessage":1349,"Filesystem":1193,"ArcanistLinter":1456,"ArcanistLintSeverity":1534}},"xmap":{"ClangFormatLinter":["ArcanistExternalLinter"]}},"d60c8224f471e0ecddc2a6f3c6839cd1":{"have":{"class":{"AutoPEP8FormatLinter":75}},"need":{"function":{"pht":297,"id":1317},"class":{"ArcanistExternalLinter":104,"ArcanistLintMessage":1324,"Filesystem":1168,"ArcanistLinter":1431,"ArcanistLintSeverity":1509}},"xmap":{"AutoPEP8FormatLinter":["ArcanistExternalLinter"]}}} \ No newline at end of file +{"__symbol_cache_version__":11,"d60c8224f471e0ecddc2a6f3c6839cd1":{"have":{"class":{"AutoPEP8FormatLinter":75}},"need":{"function":{"pht":297,"id":1317},"class":{"ArcanistExternalLinter":104,"ArcanistLintMessage":1324,"Filesystem":1168,"ArcanistLinter":1431,"ArcanistLintSeverity":1509}},"xmap":{"AutoPEP8FormatLinter":["ArcanistExternalLinter"]}},"213c3145da34ed6dfc0d70d628a2a086":{"have":{"class":{"CheckDocLinter":106}},"need":{"function":{"pht":323,"id":1868},"class":{"ArcanistExternalLinter":129,"ArcanistLintMessage":1875,"Filesystem":737,"ArcanistLinter":1923,"ArcanistLintSeverity":2009}},"xmap":{"CheckDocLinter":["ArcanistExternalLinter"]}},"7bab1f879b8a86dd9977b8c0d075935f":{"have":{"class":{"ClangFormatLinter":79}},"need":{"function":{"pht":302,"execx":787,"id":1664},"class":{"ArcanistExternalLinter":105,"ArcanistLintMessage":1671,"Filesystem":1515,"ArcanistLinter":1778,"ArcanistLintSeverity":1856}},"xmap":{"ClangFormatLinter":["ArcanistExternalLinter"]}},"d5d477360f3d8e940182368e01342fc7":{"have":{"class":{"TestsLinter":106}},"need":{"function":{"pht":321,"id":1943},"class":{"ArcanistExternalLinter":126,"ArcanistLintMessage":1950,"Filesystem":794,"ArcanistLinter":1998,"ArcanistLintSeverity":2084}},"xmap":{"TestsLinter":["ArcanistExternalLinter"]}}} \ No newline at end of file diff --git a/arcanist/__phutil_library_map__.php b/arcanist/__phutil_library_map__.php index 7dba65340..6dd7e8237 100644 --- a/arcanist/__phutil_library_map__.php +++ b/arcanist/__phutil_library_map__.php @@ -1,22 +1,24 @@ 2, 'class' => array( 'AutoPEP8FormatLinter' => 'linter/AutoPEP8Linter.php', 'CheckDocLinter' => 'linter/CheckDocLinter.php', 'ClangFormatLinter' => 'linter/ClangFormatLinter.php', + 'TestsLinter' => 'linter/TestsLinter.php', ), 'function' => array(), 'xmap' => array( 'AutoPEP8FormatLinter' => 'ArcanistExternalLinter', 'CheckDocLinter' => 'ArcanistExternalLinter', 'ClangFormatLinter' => 'ArcanistExternalLinter', + 'TestsLinter' => 'ArcanistExternalLinter', ), )); diff --git a/arcanist/linter/TestsLinter.php b/arcanist/linter/TestsLinter.php new file mode 100644 index 000000000..8548fd371 --- /dev/null +++ b/arcanist/linter/TestsLinter.php @@ -0,0 +1,151 @@ +getProjectRoot()); + } + + public function shouldUseInterpreter() { + return true; + } + + public function getDefaultInterpreter() { + return "bash"; + } + + public function getInstallInstructions() { + return pht('The test/lint/lint-tests.sh script is part of the bitcoin-abc '. + 'project'); + } + + public function shouldExpectCommandErrors() { + return false; + } + + protected function getMandatoryFlags() { + return array( + ); + } + + protected function parseLinterOutput($path, $err, $stdout, $stderr) { + /* + * Stdout contains 2 sections: + * 1/ Section with name mismatches, in the form: + * :BOOST_FIXTURE_TEST_SUITE(, ... + * 2/ Section with duplicated test names: + * + */ + + /* + * Extract infos from the path + * + * Note: the files are already filtered by path thanks to the .arclint + * configuration. If the file is not a test, the grep will find nothing and + * there will be no error to parse. + */ + $pathinfo = pathinfo($path); + $testName = $pathinfo['filename']; + $fileName = $pathinfo['basename']; + + $messages = []; + + /* Search for mismatch, using the line pattern */ + $pattern = '/'.$fileName.':BOOST_FIXTURE_TEST_SUITE\(([\w]+)/'; + $mismatch = preg_match($pattern, $stdout, $matches); + + if ($mismatch) { + /* + * Expect a single result as we are testing against a single file. + * - $matches[0] contains the full mask + * - $matches[1] contains the captured match + */ + if (count($matches) != 2) { + throw new Exception( + pht('Found multiple matches for a single file, lint-tests.sh output '. + 'is not formatted as expected, aborting.')); + } + + $mismatchName = $matches[1]; + $messages[] = id(new ArcanistLintMessage()) + ->setGranularity(ArcanistLinter::GRANULARITY_FILE) + ->setCode('TESTS') + ->setPath($path) + ->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR) + ->setName('Name mismatch') + ->setDescription( + 'The Boost test suite name must match the file name (set to "'. + $mismatchName.'", should be "'.$testName.'").'); + } + + /* + * Search for unicity, searching for test name alone on its line. + * The test name can be whether the one extracted from the file name or the + * one extracted from the BOOST_FIXTURE_TEST_SUITE content. + */ + if ($mismatch) { + $pattern = '/^'.$testName.'|'.$mismatchName.'$/'; + } else { + $pattern = '/^'.$testName.'$/'; + } + + $notUnique = preg_match($pattern, $stdout, $matches); + + /* + * Do not check the number of matches here. + * Because it is a test against unicity, there is a global search which can + * possibly return an output matching our expected test name AND our actual + * test name. This would be weird, but not impossible. + * Just returning an error for the first one is enough, as the linter will + * be rerun after the name is fix and the other match will then eventually + * get catched. + */ + + if ($notUnique) { + $messages[] = id(new ArcanistLintMessage()) + ->setGranularity(ArcanistLinter::GRANULARITY_FILE) + ->setCode('TESTS') + ->setPath($path) + ->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR) + ->setName('Duplicated name') + ->setDescription('The test name "'.$matches[0].'" already exists'); + } + + return $messages; + } +} + +?> diff --git a/test/lint/lint-tests.sh b/test/lint/lint-tests.sh index ab9532477..0581e85b5 100755 --- a/test/lint/lint-tests.sh +++ b/test/lint/lint-tests.sh @@ -1,41 +1,34 @@ #!/usr/bin/env bash # # Copyright (c) 2018 The Bitcoin Core developers # Copyright (c) 2019 The Bitcoin developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. # # Check the test suite naming conventions -EXIT_CODE=0 - TOPDIR=${TOPDIR:-$(git rev-parse --show-toplevel)} NAMING_INCONSISTENCIES=$(git grep -E '^BOOST_FIXTURE_TEST_SUITE\(' -- \ - "${TOPDIR}/src/test/**.cpp" \ - "${TOPDIR}/src/rpc/test/**.cpp" \ - "${TOPDIR}/src/wallet/test/**.cpp" | \ - grep -vE '/(.*?)\.cpp:BOOST_FIXTURE_TEST_SUITE\(\1, .*\)$') + "${1}" | grep -vE '/(.*?)\.cpp:BOOST_FIXTURE_TEST_SUITE\(\1, .*\)$') if [[ ${NAMING_INCONSISTENCIES} != "" ]]; then echo "The test suite in file src/test/foo_tests.cpp should be named" echo "\"foo_tests\". Please make sure the following test suites follow" echo "that convention:" echo echo "${NAMING_INCONSISTENCIES}" - EXIT_CODE=1 fi TEST_SUITE_NAME_COLLISIONS=$(git grep -E '^BOOST_FIXTURE_TEST_SUITE\(' -- \ "${TOPDIR}/src/test/**.cpp" \ "${TOPDIR}/src/rpc/test/**.cpp" \ "${TOPDIR}/src/wallet/test/**.cpp" | cut -f2 -d'(' | cut -f1 -d, | \ sort | uniq -d) if [[ ${TEST_SUITE_NAME_COLLISIONS} != "" ]]; then echo "Test suite names must be unique. The following test suite names" echo "appear to be used more than once:" echo echo "${TEST_SUITE_NAME_COLLISIONS}" - EXIT_CODE=1 fi -exit ${EXIT_CODE} +exit 0