Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Conversation

svenski123
Copy link
Contributor

Problem

Clippy warnings as per #9951

Summary of Changes

Lots of little clippy changes

@mvines mvines added the CI Pull Request is ready to enter CI label May 14, 2020
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label May 14, 2020
@mvines
Copy link
Contributor

mvines commented May 14, 2020

wow that really cleaned up a lot

@mvines
Copy link
Contributor

mvines commented May 14, 2020

cargo fmt please

@ryoqun ryoqun added the CI Pull Request is ready to enter CI label May 14, 2020
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label May 14, 2020
@ryoqun
Copy link
Contributor

ryoqun commented May 14, 2020

@svenski123 Thanks for working on this! I personally like these codebase hygiene maintenance. :)

@mvines mvines added the CI Pull Request is ready to enter CI label May 14, 2020
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label May 14, 2020
@svenski123
Copy link
Contributor Author

Regarding the failed test_bank_update_rewards test, there appears to be a non-deterministic element to it - running the test repeatedly usually succeeds, until it doesn't.

I doubt this relates to the clippy related changes, though I have not gone through this test case thoroughly.

$ while target/debug/deps/solana_runtime-743eb33fcc895190 test_bank_update_rewards; do true; done

running 1 test
test bank::tests::test_bank_update_rewards ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 320 filtered out

[... nine further successes ...]

running 1 test
test bank::tests::test_bank_update_rewards ... FAILED

failures:

---- bank::tests::test_bank_update_rewards stdout ----
thread 'bank::tests::test_bank_update_rewards' panicked at 'attempt to subtract with overflow', runtime/src/bank.rs:4200:15
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    bank::tests::test_bank_update_rewards

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 320 filtered out

@ryoqun
Copy link
Contributor

ryoqun commented May 15, 2020

@svenski123

Regarding the failed test_bank_update_rewards test, there appears to be a non-deterministic element to it - running the test repeatedly usually succeeds, until it doesn't.

Oops, sorry about this. This is recently fixed on master: #10031 Could you rebase this pr on it?

@ryoqun
Copy link
Contributor

ryoqun commented May 15, 2020

Hooray! I retied and the CI is running, passed coverage. Hope it finishes. :)

@codecov
Copy link

codecov bot commented May 15, 2020

Codecov Report

Merging #10030 into master will decrease coverage by 0.0%.
The diff coverage is 90.4%.

@@            Coverage Diff            @@
##           master   #10030     +/-   ##
=========================================
- Coverage    81.5%    81.5%   -0.1%     
=========================================
  Files         283      283             
  Lines       65782    65782             
=========================================
- Hits        53660    53658      -2     
- Misses      12122    12124      +2     

@ryoqun
Copy link
Contributor

ryoqun commented May 15, 2020

@svenski123 The CI passed now! But unfortunately, another big pr got landed before us.... #9992
And there are some conflicts.

However, this is getting closer to merge really! Could you rebase on that?

@svenski123 svenski123 force-pushed the 9951-clippy-errors-in-the-test-suite branch from be8d151 to f43e333 Compare May 15, 2020 12:11
@svenski123
Copy link
Contributor Author

The PR has been rebased.
cargo check and cargo clippy --tests are clean.
100 consecutive iterations of test_bank_update_rewards succeeded with no failures.
Will leave other tests for CI.

@ryoqun ryoqun added the CI Pull Request is ready to enter CI label May 15, 2020
@ryoqun
Copy link
Contributor

ryoqun commented May 15, 2020

The PR has been rebased.
cargo check and cargo clippy --tests are clean.

@svenski123 Wow. What a stamina you have! Legend thanks!! So, are you confident and ready to receive my dearest honor of adding --tests here?:

_ cargo +"$rust_stable" clippy --all --exclude solana-sdk-c -- --deny=warnings

@ryoqun ryoqun added CI Pull Request is ready to enter CI and removed CI Pull Request is ready to enter CI labels May 15, 2020
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label May 15, 2020
@mvines mvines added the automerge Merge this Pull Request automatically once CI passes label May 15, 2020
@solana-grimes solana-grimes merged commit 58ef02f into solana-labs:master May 15, 2020
@mvines
Copy link
Contributor

mvines commented May 15, 2020

@svenski123 - thanks!

@ryoqun
Copy link
Contributor

ryoqun commented May 15, 2020

@svenski123 Really thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants