From ee387875809c5fb26fac7a4e9ae648900ccdefca Mon Sep 17 00:00:00 2001 From: Peter Tseng Date: Sun, 4 Mar 2018 03:27:38 +0000 Subject: [PATCH 1/7] GitHub Actions: Run Clippy on tests --- .github/workflows/tests.yml | 14 ++++++++++++++ _test/check-exercises.sh | 4 ++-- bin/test-exercise | 17 +++++++++++++++-- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0b217d738..5f9d7f43c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -99,6 +99,9 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} run: echo "TRAVIS_PULL_REQUEST=${PR_NUMBER:-false}" >> $GITHUB_ENV + - name: Install Clippy + run: rustup component add clippy + # run scripts as steps # TODO: the TRAVIS_PULL_REQUEST variable is a holdover from before the # migration to GitHub Actions. The scripts that use it do so in order to @@ -121,6 +124,17 @@ jobs: run: sh ./_test/check-exercise-crate.sh continue-on-error: ${{ matrix.rust == 'beta' && matrix.deny_warnings == '1' }} + - name: Clippy tests + env: + CLIPPY: tests/ + run: ./_test/check-exercises.sh + + # Our examples currently have too much Clippy output to consider this. + # But maybe in the future. + #- name: Clippy examples + # env: + # CLIPPY: src/lib.rs + # run: ./_test/check-exercises.sh nightly-compilation: name: Check exercises on nightly (benchmark enabled) runs-on: ubuntu-latest diff --git a/_test/check-exercises.sh b/_test/check-exercises.sh index 0ea61e2b7..55a401196 100755 --- a/_test/check-exercises.sh +++ b/_test/check-exercises.sh @@ -11,9 +11,9 @@ if [ ! -x "./bin/test-exercise" ]; then exit 1 fi -# In DENYWARNINGS mode, do not set -e so that we run all tests. +# In DENYWARNINGS or CLIPPY mode, do not set -e so that we run all tests. # This allows us to see all warnings. -if [ -z "$DENYWARNINGS" ]; then +if [ -z "$DENYWARNINGS$CLIPPY" ]; then set -e fi diff --git a/bin/test-exercise b/bin/test-exercise index cb42f67a3..d54470066 100755 --- a/bin/test-exercise +++ b/bin/test-exercise @@ -82,6 +82,19 @@ done # (use subshell so we auto-reset to current pwd after) ( cd $exercise - # this is the last command; its exit code is what's passed on - time cargo $command "$@" + if [ -n "$CLIPPY" ]; then + cargo clippy --all-targets --color always "$@" 2>clippy.log + # Only consider it a failure if output contains the string in CLIPPY. + # For example, if we're only looking in tests, CLIPPY contains tests/ + if grep -q $CLIPPY clippy.log; then + cat clippy.log + exit 1 + else + # Just to show progress + echo "clippy $exercise OK" + fi + else + # this is the last command; its exit code is what's passed on + time cargo $command "$@" + fi ) From dbed2750204a679443c81642015f6760d24c07f5 Mon Sep 17 00:00:00 2001 From: Peter Tseng Date: Tue, 17 Nov 2020 14:08:33 +0000 Subject: [PATCH 2/7] intentionally break compilation just want to make sure the exit-code propagation still works --- exercises/accumulate/tests/accumulate.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/exercises/accumulate/tests/accumulate.rs b/exercises/accumulate/tests/accumulate.rs index 0cfb6a184..f466a8bcb 100644 --- a/exercises/accumulate/tests/accumulate.rs +++ b/exercises/accumulate/tests/accumulate.rs @@ -1,5 +1,7 @@ use accumulate::map; +this doesn't compile. + fn square(x: i32) -> i32 { x * x } From 85508c9d1a41abf0b5bf220896c4397c5a8a5026 Mon Sep 17 00:00:00 2001 From: Peter Tseng Date: Tue, 17 Nov 2020 14:16:40 +0000 Subject: [PATCH 3/7] Revert "intentionally break compilation" This reverts commit dbed2750204a679443c81642015f6760d24c07f5. --- exercises/accumulate/tests/accumulate.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/exercises/accumulate/tests/accumulate.rs b/exercises/accumulate/tests/accumulate.rs index f466a8bcb..0cfb6a184 100644 --- a/exercises/accumulate/tests/accumulate.rs +++ b/exercises/accumulate/tests/accumulate.rs @@ -1,7 +1,5 @@ use accumulate::map; -this doesn't compile. - fn square(x: i32) -> i32 { x * x } From 48853812cc7214fba01d3c52c660a50a3438b073 Mon Sep 17 00:00:00 2001 From: Peter Tseng Date: Tue, 17 Nov 2020 14:30:23 +0000 Subject: [PATCH 4/7] _test/check-exercises.sh: be more explicit in OR DENYWARNINGS or CLIPPY Co-authored-by: Peter Goodspeed-Niklaus --- _test/check-exercises.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/_test/check-exercises.sh b/_test/check-exercises.sh index 55a401196..6aa45da79 100755 --- a/_test/check-exercises.sh +++ b/_test/check-exercises.sh @@ -13,7 +13,8 @@ fi # In DENYWARNINGS or CLIPPY mode, do not set -e so that we run all tests. # This allows us to see all warnings. -if [ -z "$DENYWARNINGS$CLIPPY" ]; then +# If we are in neither DENYWARNINGS nor CLIPPY mode, do set -e. +if [ -z "$DENYWARNINGS" ] && [ -z "$CLIPPY" ]; then set -e fi From 9a81f9f795e0819c6c35205f6447937547a43ed9 Mon Sep 17 00:00:00 2001 From: Peter Tseng Date: Tue, 17 Nov 2020 14:34:26 +0000 Subject: [PATCH 5/7] GitHub Actions: move clippy to own section --- .github/workflows/tests.yml | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5f9d7f43c..fd99bf274 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -99,9 +99,6 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} run: echo "TRAVIS_PULL_REQUEST=${PR_NUMBER:-false}" >> $GITHUB_ENV - - name: Install Clippy - run: rustup component add clippy - # run scripts as steps # TODO: the TRAVIS_PULL_REQUEST variable is a holdover from before the # migration to GitHub Actions. The scripts that use it do so in order to @@ -124,6 +121,35 @@ jobs: run: sh ./_test/check-exercise-crate.sh continue-on-error: ${{ matrix.rust == 'beta' && matrix.deny_warnings == '1' }} + clippy: + name: Clippy + runs-on: ubuntu-latest + + strategy: + matrix: + rust: ["stable", "beta"] + + steps: + - name: Checkout master + uses: actions/checkout@v2 + with: + ref: master + + - name: Checkout code + uses: actions/checkout@v2 + + # We actually think explicitly installing Clippy isn't necessary, + # as it is currently installed by default. + # But we'll include this step anyway just in case that changes. + - name: Install Clippy + run: rustup component add clippy + + - name: Setup toolchain + uses: actions-rs/toolchain@v1 + with: + toolchain: ${{ matrix.rust }} + default: true + - name: Clippy tests env: CLIPPY: tests/ From 615aa01d642aca9c7aa8942293728cc978e78667 Mon Sep 17 00:00:00 2001 From: Peter Tseng Date: Tue, 17 Nov 2020 14:20:48 +0000 Subject: [PATCH 6/7] violate clippy by adding return --- exercises/protein-translation/tests/proteins.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exercises/protein-translation/tests/proteins.rs b/exercises/protein-translation/tests/proteins.rs index 828efab00..ca7ed0c3a 100644 --- a/exercises/protein-translation/tests/proteins.rs +++ b/exercises/protein-translation/tests/proteins.rs @@ -153,5 +153,5 @@ fn make_pairs() -> Vec<(&'static str, &'static str)> { } } pairs.sort_by(|&(_, a), &(_, b)| a.cmp(b)); - pairs + return pairs; } From 5bd7203c815ed5a7853c620976b7c759330866b6 Mon Sep 17 00:00:00 2001 From: Peter Tseng Date: Tue, 17 Nov 2020 14:54:55 +0000 Subject: [PATCH 7/7] Revert "violate clippy by adding return" This reverts commit 615aa01d642aca9c7aa8942293728cc978e78667. --- exercises/protein-translation/tests/proteins.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exercises/protein-translation/tests/proteins.rs b/exercises/protein-translation/tests/proteins.rs index ca7ed0c3a..828efab00 100644 --- a/exercises/protein-translation/tests/proteins.rs +++ b/exercises/protein-translation/tests/proteins.rs @@ -153,5 +153,5 @@ fn make_pairs() -> Vec<(&'static str, &'static str)> { } } pairs.sort_by(|&(_, a), &(_, b)| a.cmp(b)); - return pairs; + pairs }