Page MenuHomePhabricator
Files F13115322

D2533.diff
No OneTemporary
Actions

D2533.diff
View Options

diff --git a/.arclint b/.arclint
--- a/.arclint
+++ b/.arclint
@@ -35,6 +35,10 @@
"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
--- 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
--- a/arcanist/__phutil_library_map__.php
+++ b/arcanist/__phutil_library_map__.php
@@ -12,11 +12,13 @@
'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
--- /dev/null
+++ b/arcanist/linter/TestsLinter.php
@@ -0,0 +1,151 @@
+<?php
+
+/**
+ * Uses the lint-tests.sh script to enfore Boost unit tests name conformity
+ */
+final class TestsLinter extends ArcanistExternalLinter {
+
+ public function getInfoName() {
+ return 'lint-tests';
+ }
+
+ public function getInfoURI() {
+ return '';
+ }
+
+ public function getInfoDescription() {
+ return pht('Check the unit tests for duplicates and ensure the file name '.
+ 'matches the boost test suite name.');
+ }
+
+ public function getLinterName() {
+ return 'lint-tests';
+ }
+
+ public function getLinterConfigurationName() {
+ return 'lint-tests';
+ }
+
+ public function getLinterConfigurationOptions() {
+ $options = array(
+ );
+
+ return $options + parent::getLinterConfigurationOptions();
+ }
+
+ public function getDefaultBinary() {
+ return Filesystem::resolvePath(
+ 'test/lint/lint-tests.sh',
+ $this->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:
+ * <file path>:BOOST_FIXTURE_TEST_SUITE(<test name>, ...
+ * 2/ Section with duplicated test names:
+ * <test_name>
+ */
+
+ /*
+ * 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
--- a/test/lint/lint-tests.sh
+++ b/test/lint/lint-tests.sh
@@ -7,22 +7,16 @@
#
# 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\(' -- \
@@ -35,7 +29,6 @@
echo "appear to be used more than once:"
echo
echo "${TEST_SUITE_NAME_COLLISIONS}"
- EXIT_CODE=1
fi
-exit ${EXIT_CODE}
+exit 0

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 1, 10:50 (11 h, 56 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5184660
Default Alt Text
D2533.diff (8 KB)

Event Timeline