Skip to content

Commit

Permalink
cleanup: Enable clippy for tests, benches, examples, etc (#8931)
Browse files Browse the repository at this point in the history
Test code should be well-written, maintainable and follow best practices.
  • Loading branch information
nikurt authored Apr 21, 2023
1 parent 75e36d3 commit 83540d7
Show file tree
Hide file tree
Showing 31 changed files with 79 additions and 91 deletions.
5 changes: 2 additions & 3 deletions chain/client/src/tests/bug_repros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,10 @@ fn repro_1183() {
if thread_rng().gen_bool(0.5) {
(NetworkResponses::NoResponse.into(), true)
} else {
let msg2 = msg.clone();
delayed_one_parts
.write()
.unwrap()
.push(msg2.as_network_requests_ref().clone());
.push(msg.as_network_requests_ref().clone());
(NetworkResponses::NoResponse.into(), false)
}
} else {
Expand Down Expand Up @@ -226,7 +225,7 @@ fn test_sync_from_archival_node() {
(NetworkResponses::NoResponse.into(), false)
}
NetworkRequests::Approval { approval_message } => {
for (i, actor_handles) in conns.clone().into_iter().enumerate() {
for (i, actor_handles) in conns.into_iter().enumerate() {
if i != 3 {
actor_handles.client_actor.do_send(
BlockApproval(
Expand Down
6 changes: 3 additions & 3 deletions chain/network/src/peer_manager/tests/routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1293,12 +1293,12 @@ async fn archival_node() {
let pm0_connections: HashSet<PeerId> =
pm0.with_state(|s| async move { s.tier2.load().ready.keys().cloned().collect() }).await;

let chosen = vec![&pm2, &pm3, &pm4]
let pms = vec![&pm2, &pm3, &pm4];
let chosen = pms
.iter()
.filter(|&pm| !pm0_connections.contains(&pm.cfg.node_id()))
.choose(rng)
.unwrap()
.clone();
.unwrap();

tracing::info!(target:"test", "[{_step}] wait for the chosen node to finish disconnecting from node 0");
chosen.wait_for_num_connected_peers(0).await;
Expand Down
2 changes: 1 addition & 1 deletion core/async/src/examples/async_component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl OuterComponent {
}

pub fn process_request(&mut self, request: OuterRequest, future_spawner: &dyn FutureSpawner) {
let inner_request = InnerRequest(request.0.clone());
let inner_request = InnerRequest(request.0);
let sender = self.inner_sender.clone();
let response_sender = self.outer_response_sender.clone();

Expand Down
2 changes: 1 addition & 1 deletion core/async/src/examples/sum_numbers_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ fn test_simple_with_adhoc() {
// executed in the TestLoop context. This allows the setup logic to show
// up in the visualizer too, with any of its logging shown under the
// adhoc event.
let sender = test.sender().clone();
let sender = test.sender();
test.sender().send_adhoc_event("initial events", move |_| {
sender.send(SumRequest::Number(1));
sender.send(SumRequest::Number(2));
Expand Down
2 changes: 2 additions & 0 deletions core/o11y/src/io_tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ impl<S: Subscriber + for<'span> LookupSpan<'span>> Layer<S> for IoTraceLayer {
}
}

#[allow(clippy::integer_arithmetic)]
fn on_exit(&self, id: &span::Id, ctx: tracing_subscriber::layer::Context<'_, S>) {
// When the span exits, produce one line for the span itself that
// includes key=value pairs from `SpanInfo`. Then also add indentation
Expand Down Expand Up @@ -301,6 +302,7 @@ impl tracing::field::Visit for IoEventVisitor {
}

impl tracing::field::Visit for SpanInfo {
#[allow(clippy::integer_arithmetic)]
fn record_str(&mut self, field: &tracing::field::Field, value: &str) {
// "count" is a special field, everything else are key values pairs.
if field.name() == "counter" {
Expand Down
2 changes: 1 addition & 1 deletion core/primitives/src/trie_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ mod tests {
assert_eq!(
TrieKey::ContractData { account_id: account_id.clone(), key: Default::default() }
.get_account_id(),
Some(account_id.clone())
Some(account_id)
);
}
}
2 changes: 1 addition & 1 deletion core/store/benches/store_bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn write_to_db(store: &Store, keys: &[Vec<u8>], max_value_size: usize, col: DBCo
let x: usize = rand::random::<usize>() % max_value_size;
let val: Vec<u8> = (0..x).map(|_| rand::random::<u8>()).collect();
// NOTE: this
store_update.set(col, key.as_slice().clone(), &val);
store_update.set(col, &key, &val);
}
store_update.commit().unwrap();
}
Expand Down
2 changes: 1 addition & 1 deletion core/store/src/db/rocksdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ mod tests {
let mut result = StoreStatistics { data: vec![] };
let parse_result = parse_statistics(statistics, &mut result);
// We should be able to parse stats and the result should be Ok(()).
assert_eq!(parse_result.unwrap(), ());
parse_result.unwrap();
assert_eq!(
result,
StoreStatistics {
Expand Down
6 changes: 3 additions & 3 deletions core/store/src/flat/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,15 +374,15 @@ mod tests {
.iter()
.cloned()
.map(|height| {
let hash = height_to_hashes.get(&height).unwrap().clone();
let hash = *height_to_hashes.get(&height).unwrap();
let prev_hash = match get_parent(height) {
None => CryptoHash::default(),
Some(parent_height) => *height_to_hashes.get(&parent_height).unwrap(),
};
(hash, BlockInfo { hash, height, prev_hash })
})
.collect();
MockChain { height_to_hashes, blocks, head_height: heights.last().unwrap().clone() }
MockChain { height_to_hashes, blocks, head_height: *heights.last().unwrap() }
}

// Create a chain with no forks with length n.
Expand Down Expand Up @@ -526,7 +526,7 @@ mod tests {

// Check that flat storage state is created correctly for chain which has skipped heights.
let flat_storage = FlatStorage::new(store.clone(), shard_uid);
let flat_storage_manager = FlatStorageManager::new(store.clone());
let flat_storage_manager = FlatStorageManager::new(store);
flat_storage_manager.add_flat_storage_for_shard(shard_uid, flat_storage);
let flat_storage = flat_storage_manager.get_flat_storage_for_shard(shard_uid).unwrap();

Expand Down
2 changes: 1 addition & 1 deletion core/store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,7 @@ mod tests {
assert_eq!(false, cache.has(&key).unwrap());

let record = CompiledContract::Code(b"foo".to_vec());
assert_eq!((), cache.put(&key, record.clone()).unwrap());
cache.put(&key, record.clone()).unwrap();
assert_eq!(Some(record), cache.get(&key).unwrap());
assert_eq!(true, cache.has(&key).unwrap());
}
Expand Down
2 changes: 1 addition & 1 deletion core/store/src/trie/shard_tries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ mod test {
let block_hash =
CryptoHash::from_str("32222222222233333333334444444444445555555777").unwrap();

for trie_key in [trie_key1.clone(), trie_key2.clone()] {
for trie_key in [trie_key1.clone(), trie_key2] {
let row_key = KeyForStateChanges::delayed_receipt_key_from_trie_key(
&block_hash,
&trie_key,
Expand Down
2 changes: 1 addition & 1 deletion core/store/src/trie/state_parts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ mod tests {
break;
}
if let Some(NodeHandle::Hash(ref h)) = children[i] {
let h = h.clone();
let h = *h;
let child = self.retrieve_node(&h)?.1;
stack.push((hash, node, CrumbStatus::AtChild(i + 1)));
stack.push((h, child, CrumbStatus::Entering));
Expand Down
23 changes: 5 additions & 18 deletions integration-tests/src/tests/client/features/delegate_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,8 @@ fn check_meta_tx_execution(
};
let user_nonce_before = node_user.get_access_key(&sender, &user_pubk).unwrap().nonce;

let tx_result = node_user
.meta_tx(sender.clone(), receiver.clone(), relayer.clone(), actions.clone())
.unwrap();
let tx_result =
node_user.meta_tx(sender.clone(), receiver.clone(), relayer.clone(), actions).unwrap();

// Execution of the transaction and all receipts should succeed
tx_result.assert_success();
Expand Down Expand Up @@ -449,15 +448,7 @@ fn meta_tx_stake() {
let tx_cost = fee_helper.stake_cost();
let public_key = create_user_test_signer(&sender).public_key;
let actions = vec![Action::Stake(StakeAction { public_key, stake: 0 })];
check_meta_tx_no_fn_call(
&node,
actions,
tx_cost,
0,
sender.clone(),
relayer.clone(),
receiver.clone(),
);
check_meta_tx_no_fn_call(&node, actions, tx_cost, 0, sender, relayer, receiver);
}

#[test]
Expand Down Expand Up @@ -793,10 +784,7 @@ fn meta_tx_create_implicit_account_fails() {
let node = RuntimeNode::new(&relayer);

let actions = vec![Action::CreateAccount(CreateAccountAction {})];
let tx_result = node
.user()
.meta_tx(sender.clone(), new_account.clone(), relayer.clone(), actions.clone())
.unwrap();
let tx_result = node.user().meta_tx(sender, new_account, relayer, actions).unwrap();

let account_creation_result = &tx_result.receipts_outcome[1].outcome.status;
assert!(matches!(
Expand Down Expand Up @@ -831,8 +819,7 @@ fn meta_tx_create_and_use_implicit_account() {

// Execute and expect `AccountDoesNotExist`, as we try to call a meta
// transaction on a user that doesn't exist yet.
let tx_result =
node.user().meta_tx(sender.clone(), new_account.clone(), relayer.clone(), actions).unwrap();
let tx_result = node.user().meta_tx(sender, new_account.clone(), relayer, actions).unwrap();
let status = &tx_result.receipts_outcome[1].outcome.status;
assert!(matches!(
status,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn prepare_env_with_contract(
let mut env = TestEnv::builder(ChainGenesis::new(&genesis))
.runtime_adapters(create_nightshade_runtimes(&genesis, 1))
.build();
deploy_test_contract(&mut env, account, &contract, epoch_length.clone(), 1);
deploy_test_contract(&mut env, account, &contract, epoch_length, 1);
env
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,10 @@ fn test_zero_balance_account_creation() {
let create_account_tx = SignedTransaction::create_contract(
2,
signer0.account_id.clone(),
new_account_id.clone(),
new_account_id,
contract.to_vec(),
0,
new_signer.public_key.clone(),
new_signer.public_key,
&signer0,
*genesis_block.hash(),
);
Expand Down Expand Up @@ -218,7 +218,7 @@ fn test_zero_balance_account_add_key() {
let send_money_tx = SignedTransaction::send_money(
nonce + 10,
new_account_id.clone(),
signer0.account_id.clone(),
signer0.account_id,
&new_signer,
amount,
*genesis_block.hash(),
Expand Down Expand Up @@ -295,9 +295,9 @@ fn test_zero_balance_account_upgrade() {
let create_account_tx2 = SignedTransaction::create_account(
2,
signer0.account_id.clone(),
new_account_id.clone(),
new_account_id,
0,
new_signer.public_key.clone(),
new_signer.public_key,
&signer0,
*genesis_block.hash(),
);
Expand Down
8 changes: 4 additions & 4 deletions integration-tests/src/tests/client/flat_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,9 @@ fn test_flat_storage_creation_sanity() {
);
// Introduce fork block to check that deltas for it will be GC-d later.
let fork_block = env.clients[0].produce_block(START_HEIGHT + 2).unwrap().unwrap();
let fork_block_hash = fork_block.hash().clone();
let fork_block_hash = *fork_block.hash();
let next_block = env.clients[0].produce_block(START_HEIGHT + 3).unwrap().unwrap();
let next_block_hash = next_block.hash().clone();
let next_block_hash = *next_block.hash();
env.process_block(0, fork_block, Provenance::PRODUCED);
env.process_block(0, next_block, Provenance::PRODUCED);

Expand Down Expand Up @@ -388,7 +388,7 @@ fn test_flat_storage_creation_start_from_state_part() {
store_update.commit().unwrap();

// Re-create runtime, check that flat storage is not created yet.
let mut env = setup_env(&genesis, store.clone());
let mut env = setup_env(&genesis, store);
assert!(env.clients[0].runtime_adapter.get_flat_storage_for_shard(shard_uid).is_none());

// Run chain for a couple of blocks and check that flat storage for shard 0 is eventually created.
Expand Down Expand Up @@ -526,7 +526,7 @@ fn test_not_supported_block() {
let shard_uid = shard_layout.get_shard_uids()[0];
let store = create_test_store();

let mut env = setup_env(&genesis, store.clone());
let mut env = setup_env(&genesis, store);
// Produce blocks up to `START_HEIGHT`.
for height in 1..START_HEIGHT {
env.produce_block(0, height);
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/src/tests/client/process_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,7 @@ fn ban_peer_for_invalid_block_common(mode: InvalidBlockMode) {
}
}

for (i, actor_handles) in conns.clone().into_iter().enumerate() {
for (i, actor_handles) in conns.into_iter().enumerate() {
if i != block_producer_idx {
actor_handles.client_actor.do_send(
BlockResponse {
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/src/tests/client/undo_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn test_undo_block(epoch_length: u64, stop_height: u64) {
assert_eq!(chain_store.head().unwrap().height, stop_height - 1);

// set up an environment again with the same store
let (mut env, _) = setup_env(&genesis, store.clone());
let (mut env, _) = setup_env(&genesis, store);
// the new env should be able to produce block normally
let block = env.clients[0].produce_block(stop_height).unwrap().unwrap();
env.process_block(0, block, Provenance::PRODUCED);
Expand Down
6 changes: 3 additions & 3 deletions runtime/near-vm-logic/src/gas_counter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ mod tests {
)
.unwrap();

let mut profile = counter.profile_data().clone();
let mut profile = counter.profile_data();
profile.compute_wasm_instruction_cost(counter.burnt_gas());

assert_eq!(profile.total_compute_usage(&ExtCostsConfig::test()), counter.burnt_gas());
Expand All @@ -379,7 +379,7 @@ mod tests {
counter.pay_per(near_primitives::config::ExtCosts::storage_write_value_byte, 100),
want.map_err(Into::into)
);
let mut profile = counter.profile_data().clone();
let mut profile = counter.profile_data();
profile.compute_wasm_instruction_cost(counter.burnt_gas());

assert_eq!(profile.total_compute_usage(&ExtCostsConfig::test()), counter.burnt_gas());
Expand All @@ -402,7 +402,7 @@ mod tests {
),
want.map_err(Into::into)
);
let mut profile = counter.profile_data().clone();
let mut profile = counter.profile_data();
profile.compute_wasm_instruction_cost(counter.burnt_gas());

assert_eq!(profile.total_compute_usage(&ExtCostsConfig::test()), counter.burnt_gas());
Expand Down
4 changes: 2 additions & 2 deletions runtime/near-vm/lib/api/src/sys/externals/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,7 @@ mod inner {

#[test]
fn test_from_array() {
assert_eq!(<()>::from_array([]), ());
<()>::from_array([]);
assert_eq!(<i32>::from_array([1]), (1i32));
assert_eq!(<(i32, i64)>::from_array([1, 2]), (1i32, 2i64));
assert_eq!(
Expand Down Expand Up @@ -1480,7 +1480,7 @@ mod inner {

#[test]
fn test_from_c_struct() {
assert_eq!(<()>::from_c_struct(S0()), ());
<()>::from_c_struct(S0());
assert_eq!(<i32>::from_c_struct(S1(1)), (1i32));
assert_eq!(<(i32, i64)>::from_c_struct(S2(1, 2)), (1i32, 2i64));
assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion runtime/near-vm/lib/api/src/sys/import_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ mod test {
"small" => g.clone()
},
"cat" => {
"small" => g.clone()
"small" => g
}
};

Expand Down
2 changes: 1 addition & 1 deletion runtime/near-vm/lib/types/src/entity/boxed_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ mod tests {

impl EntityRef for E {
fn new(i: usize) -> Self {
E(i as u32)
Self(i as u32)
}
fn index(self) -> usize {
self.0 as usize
Expand Down
4 changes: 2 additions & 2 deletions runtime/near-vm/lib/types/src/entity/packed_option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ mod tests {

impl ReservedValue for NoC {
fn reserved_value() -> Self {
NoC(13)
Self(13)
}

fn is_reserved_value(&self) -> bool {
Expand All @@ -149,7 +149,7 @@ mod tests {

impl ReservedValue for Ent {
fn reserved_value() -> Self {
Ent(13)
Self(13)
}

fn is_reserved_value(&self) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion runtime/near-vm/lib/types/src/entity/primary_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ mod tests {

impl EntityRef for E {
fn new(i: usize) -> Self {
E(i as u32)
Self(i as u32)
}
fn index(self) -> usize {
self.0 as usize
Expand Down
2 changes: 1 addition & 1 deletion runtime/near-vm/lib/types/src/entity/secondary_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ mod tests {

impl EntityRef for E {
fn new(i: usize) -> Self {
E(i as u32)
Self(i as u32)
}
fn index(self) -> usize {
self.0 as usize
Expand Down
Loading

0 comments on commit 83540d7

Please sign in to comment.