From 7be2b2456acac4ad677fdd13f4b83dc7f63c202b Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 29 May 2018 15:35:01 +0200 Subject: [PATCH] Merge #13281: test: Move linters to test/lint, add readme fa3c910bfeab00703c947c5200a64c21225b50ef test: Move linters to test/lint, add readme (MarcoFalke) Pull request description: This moves the checks and linters from `devtools` to a subfolder in `test`. (Motivated by my opinion that the dev tools are mostly for generating code and updating the repo whereas the linters are read-only checks.) Also, adds a readme to clarify that checks and linters are only meant to prevent bugs and user facing issues, not merely stylistic preference or inconsistencies. (This is motivated by the diversity in developers and work flows as well as existing code styles. It would be too disruptive to change all existing code to a single style or too burdensome to force all developers to adhere to a single style. Also note that our style guide is changing, so locking in at the wrong style "too early" would only waste resources.) Tree-SHA512: 9b10e89f2aeaf0c8a9ae248aa891d74e0abf0569f8e5dfd266446efa8bfaf19f0ea0980abf0b0b22f0d8416ee90d7435d21a9f9285b66df43f370b7979173406 --- .travis.yml | 3 ++ ci/build_src.sh | 18 +++++++++--- contrib/devtools/README.md | 17 ----------- doc/developer-notes.md | 4 +-- test/functional/README.md | 2 ++ test/lint/README.md | 29 +++++++++++++++++++ {contrib/devtools => test/lint}/check-doc.py | 0 .../lint}/check-rpc-mappings.py | 0 .../lint}/commit-script-check.sh | 0 .../lint}/git-subtree-check.sh | 0 {contrib/devtools => test/lint}/lint-all.sh | 0 .../lint}/lint-include-guards.sh | 0 .../devtools => test/lint}/lint-includes.sh | 0 {contrib/devtools => test/lint}/lint-logs.sh | 0 .../lint}/lint-python-shebang.sh | 0 .../devtools => test/lint}/lint-python.sh | 0 {contrib/devtools => test/lint}/lint-shell.sh | 0 {contrib/devtools => test/lint}/lint-tests.sh | 0 .../devtools => test/lint}/lint-whitespace.sh | 0 19 files changed, 50 insertions(+), 23 deletions(-) create mode 100644 test/lint/README.md rename {contrib/devtools => test/lint}/check-doc.py (100%) rename {contrib/devtools => test/lint}/check-rpc-mappings.py (100%) rename {contrib/devtools => test/lint}/commit-script-check.sh (100%) rename {contrib/devtools => test/lint}/git-subtree-check.sh (100%) rename {contrib/devtools => test/lint}/lint-all.sh (100%) rename {contrib/devtools => test/lint}/lint-include-guards.sh (100%) rename {contrib/devtools => test/lint}/lint-includes.sh (100%) rename {contrib/devtools => test/lint}/lint-logs.sh (100%) rename {contrib/devtools => test/lint}/lint-python-shebang.sh (100%) rename {contrib/devtools => test/lint}/lint-python.sh (100%) rename {contrib/devtools => test/lint}/lint-shell.sh (100%) rename {contrib/devtools => test/lint}/lint-tests.sh (100%) rename {contrib/devtools => test/lint}/lint-whitespace.sh (100%) diff --git a/.travis.yml b/.travis.yml index 4820ff8c2b..bf53804f15 100644 --- a/.travis.yml +++ b/.travis.yml @@ -137,6 +137,9 @@ before_script: - python3 -c 'import os,sys,fcntl; flags = fcntl.fcntl(sys.stdout, fcntl.F_GETFL); fcntl.fcntl(sys.stdout, fcntl.F_SETFL, flags&~os.O_NONBLOCK);' # Build docker image only for develop branch of the main repo - if [ "$TRAVIS_REPO_SLUG" != "dashpay/dash" -o "$TRAVIS_BRANCH" != "develop" -o "$TRAVIS_PULL_REQUEST" != "false" ]; then export DOCKER_BUILD="false"; echo DOCKER_BUILD=$DOCKER_BUILD; fi + # TODO: Check keys and signed commits + #- if [ "$TRAVIS_REPO_SLUG" = "dashpay/dash" -a "$TRAVIS_PULL_REQUEST" = "false" ]; then while read LINE; do travis_retry gpg --keyserver hkp://subset.pool.sks-keyservers.net --recv-keys $LINE; done < contrib/verify-commits/trusted-keys; fi + #- if [ "$TRAVIS_REPO_SLUG" = "dashpay/dash" -a "$TRAVIS_EVENT_TYPE" = "cron" ]; then travis_wait 30 contrib/verify-commits/verify-commits.sh; fi after_script: - echo $TRAVIS_COMMIT_RANGE - echo $TRAVIS_COMMIT_LOG diff --git a/ci/build_src.sh b/ci/build_src.sh index b60f14a866..17c51d0387 100755 --- a/ci/build_src.sh +++ b/ci/build_src.sh @@ -12,11 +12,21 @@ unset DISPLAY export CCACHE_COMPRESS=${CCACHE_COMPRESS:-1} export CCACHE_SIZE=${CCACHE_SIZE:-400M} -if [ "$PULL_REQUEST" != "false" ]; then contrib/devtools/commit-script-check.sh $COMMIT_RANGE; fi +if [ "$PULL_REQUEST" != "false" ]; then test/lint/commit-script-check.sh $COMMIT_RANGE; fi -#if [ "$CHECK_DOC" = 1 ]; then contrib/devtools/check-doc.py; fi TODO reenable after all Bitcoin PRs have been merged and docs fully fixed -if [ "$CHECK_DOC" = 1 ]; then contrib/devtools/check-rpc-mappings.py .; fi -if [ "$CHECK_DOC" = 1 ]; then contrib/devtools/lint-all.sh; fi +if [ "$CHECK_DOC" = 1 ]; then + # TODO: Verify subtrees + #test/lint/git-subtree-check.sh src/crypto/ctaes + #test/lint/git-subtree-check.sh src/secp256k1 + #test/lint/git-subtree-check.sh src/univalue + #test/lint/git-subtree-check.sh src/leveldb + # TODO: Check docs (reenable after all Bitcoin PRs have been merged and docs fully fixed) + #test/lint/check-doc.py + # Check rpc consistency + test/lint/check-rpc-mappings.py . + # Run all linters + test/lint/lint-all.sh +fi ccache --max-size=$CCACHE_SIZE diff --git a/contrib/devtools/README.md b/contrib/devtools/README.md index 343e3c8d72..535f2906e5 100644 --- a/contrib/devtools/README.md +++ b/contrib/devtools/README.md @@ -87,23 +87,6 @@ example: BUILDDIR=$PWD/build contrib/devtools/gen-manpages.sh ``` -git-subtree-check.sh -==================== - -Run this script from the root of the repository to verify that a subtree matches the contents of -the commit it claims to have been updated to. - -To use, make sure that you have fetched the upstream repository branch in which the subtree is -maintained: -* for `src/secp256k1`: https://github.com/bitcoin-core/secp256k1.git (branch master) -* for `src/leveldb`: https://github.com/bitcoin-core/leveldb.git (branch bitcoin-fork) -* for `src/univalue`: https://github.com/bitcoin-core/univalue.git (branch master) -* for `src/crypto/ctaes`: https://github.com/bitcoin-core/ctaes.git (branch master) - -Usage: `git-subtree-check.sh DIR (COMMIT)` - -`COMMIT` may be omitted, in which case `HEAD` is used. - github-merge.py =============== diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 47e0532342..21fc360bbf 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -592,7 +592,7 @@ Others are external projects without a tight relationship with our project. Cha be sent upstream but bugfixes may also be prudent to PR against Dash Core so that they can be integrated quickly. Cosmetic changes should be purely taken upstream. -There is a tool in contrib/devtools/git-subtree-check.sh to check a subtree directory for consistency with +There is a tool in `test/lint/git-subtree-check.sh` to check a subtree directory for consistency with its upstream repository. Current subtrees include: @@ -674,7 +674,7 @@ To create a scripted-diff: - `-BEGIN VERIFY SCRIPT-` - `-END VERIFY SCRIPT-` -The scripted-diff is verified by the tool `contrib/devtools/commit-script-check.sh` +The scripted-diff is verified by the tool `test/lint/commit-script-check.sh` Commit [`bb81e173`](https://github.com/bitcoin/bitcoin/commit/bb81e173) is an example of a scripted-diff. diff --git a/test/functional/README.md b/test/functional/README.md index 85600cdc88..ac1aa64d80 100644 --- a/test/functional/README.md +++ b/test/functional/README.md @@ -20,6 +20,8 @@ don't have test cases for. - Where possible, try to adhere to [PEP-8 guidelines](https://www.python.org/dev/peps/pep-0008/) - Use a python linter like flake8 before submitting PRs to catch common style nits (eg trailing whitespace, unused imports, etc) +- See [the python lint script](/test/lint/lint-python.sh) that checks for violations that + could lead to bugs and issues in the test code. - Avoid wildcard imports where possible - Use a module-level docstring to describe what the test is testing, and how it is testing it. diff --git a/test/lint/README.md b/test/lint/README.md new file mode 100644 index 0000000000..15974a3598 --- /dev/null +++ b/test/lint/README.md @@ -0,0 +1,29 @@ +This folder contains lint scripts. + +check-doc.py +============ +Check for missing documentation of command line options. + +commit-script-check.sh +====================== +Verification of [scripted diffs](/doc/developer-notes.md#scripted-diffs). + +git-subtree-check.sh +==================== +Run this script from the root of the repository to verify that a subtree matches the contents of +the commit it claims to have been updated to. + +To use, make sure that you have fetched the upstream repository branch in which the subtree is +maintained: +* for `src/secp256k1`: https://github.com/bitcoin-core/secp256k1.git (branch master) +* for `src/leveldb`: https://github.com/bitcoin-core/leveldb.git (branch bitcoin-fork) +* for `src/univalue`: https://github.com/bitcoin-core/univalue.git (branch master) +* for `src/crypto/ctaes`: https://github.com/bitcoin-core/ctaes.git (branch master) + +Usage: `git-subtree-check.sh DIR (COMMIT)` + +`COMMIT` may be omitted, in which case `HEAD` is used. + +lint-all.sh +=========== +Calls other scripts with the `lint-` prefix. diff --git a/contrib/devtools/check-doc.py b/test/lint/check-doc.py similarity index 100% rename from contrib/devtools/check-doc.py rename to test/lint/check-doc.py diff --git a/contrib/devtools/check-rpc-mappings.py b/test/lint/check-rpc-mappings.py similarity index 100% rename from contrib/devtools/check-rpc-mappings.py rename to test/lint/check-rpc-mappings.py diff --git a/contrib/devtools/commit-script-check.sh b/test/lint/commit-script-check.sh similarity index 100% rename from contrib/devtools/commit-script-check.sh rename to test/lint/commit-script-check.sh diff --git a/contrib/devtools/git-subtree-check.sh b/test/lint/git-subtree-check.sh similarity index 100% rename from contrib/devtools/git-subtree-check.sh rename to test/lint/git-subtree-check.sh diff --git a/contrib/devtools/lint-all.sh b/test/lint/lint-all.sh similarity index 100% rename from contrib/devtools/lint-all.sh rename to test/lint/lint-all.sh diff --git a/contrib/devtools/lint-include-guards.sh b/test/lint/lint-include-guards.sh similarity index 100% rename from contrib/devtools/lint-include-guards.sh rename to test/lint/lint-include-guards.sh diff --git a/contrib/devtools/lint-includes.sh b/test/lint/lint-includes.sh similarity index 100% rename from contrib/devtools/lint-includes.sh rename to test/lint/lint-includes.sh diff --git a/contrib/devtools/lint-logs.sh b/test/lint/lint-logs.sh similarity index 100% rename from contrib/devtools/lint-logs.sh rename to test/lint/lint-logs.sh diff --git a/contrib/devtools/lint-python-shebang.sh b/test/lint/lint-python-shebang.sh similarity index 100% rename from contrib/devtools/lint-python-shebang.sh rename to test/lint/lint-python-shebang.sh diff --git a/contrib/devtools/lint-python.sh b/test/lint/lint-python.sh similarity index 100% rename from contrib/devtools/lint-python.sh rename to test/lint/lint-python.sh diff --git a/contrib/devtools/lint-shell.sh b/test/lint/lint-shell.sh similarity index 100% rename from contrib/devtools/lint-shell.sh rename to test/lint/lint-shell.sh diff --git a/contrib/devtools/lint-tests.sh b/test/lint/lint-tests.sh similarity index 100% rename from contrib/devtools/lint-tests.sh rename to test/lint/lint-tests.sh diff --git a/contrib/devtools/lint-whitespace.sh b/test/lint/lint-whitespace.sh similarity index 100% rename from contrib/devtools/lint-whitespace.sh rename to test/lint/lint-whitespace.sh