Skip to content

Commit

Permalink
Clippy Dead Test Code (#804)
Browse files Browse the repository at this point in the history
* move mobile verifier integration tests to single file

Reading about dead code problems in tests, multiple people pointed to
this blog post.
https://matklad.github.io/2021/02/27/delete-cargo-integration-tests.html

The issue is, cargo compiles a single binary for each test suite. When
you try to share code across suites, the checking is done at a per-suite
level.

For example, you have:
- commons/mod.rs
- one.rs
- two.rs
```
pub fn shared_fn() {}
```

If only one.rs uses `shared_fn()`, cargo test will complain that it's
unused, because it's not used in all created binaries.

Moving all the test modules into a `integrations/main.rs` forces a
single binary to be compiled, which has the same type of dead code
checking as a regular application.

* move boost manager integration tests to single file

* remove `#[allow(dead_code)]`

* move iot verifier tests to single file

- remove `#[allow(dead_code)]`

* remove unused mock receiver
  • Loading branch information
michaeldjeffrey authored May 6, 2024
1 parent 5816910 commit 43d5a56
Show file tree
Hide file tree
Showing 26 changed files with 80 additions and 113 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
mod common;
use boost_manager::{activator, db, OnChainStatus};
use chrono::{DateTime, Duration as ChronoDuration, Duration, Timelike, Utc};
use mobile_config::boosted_hex_info::{BoostedHex, BoostedHexInfo, BoostedHexes};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,48 @@
use async_trait::async_trait;
use chrono::DateTime;
use chrono::Utc;
use file_store::file_sink::{FileSinkClient, Message as SinkMessage};
use futures_util::{stream, StreamExt as FuturesStreamExt};
use helium_proto::BoostedHexInfoV1 as BoostedHexInfoProto;
use helium_proto::BoostedHexUpdateV1 as BoostedHexUpdateProto;
use helium_proto::Message;
use mobile_config::boosted_hex_info::BoostedHexInfo;
use mobile_config::boosted_hex_info::BoostedHexInfoStream;
use mobile_config::client::hex_boosting_client::HexBoostingInfoResolver;
use mobile_config::client::ClientError;
use tokio::{sync::mpsc::error::TryRecvError, time::timeout};

#[derive(Debug, Clone)]
pub struct MockHexBoostingClient {
pub boosted_hexes: Vec<BoostedHexInfo>,
}

impl MockHexBoostingClient {
pub fn new(boosted_hexes: Vec<BoostedHexInfo>) -> Self {
Self { boosted_hexes }
}
}

#[async_trait]
impl HexBoostingInfoResolver for MockHexBoostingClient {
type Error = ClientError;

async fn stream_boosted_hexes_info(&mut self) -> Result<BoostedHexInfoStream, ClientError> {
Ok(stream::iter(self.boosted_hexes.clone()).boxed())
}

async fn stream_modified_boosted_hexes_info(
&mut self,
_timestamp: DateTime<Utc>,
) -> Result<BoostedHexInfoStream, ClientError> {
Ok(stream::iter(self.boosted_hexes.clone()).boxed())
}
}

pub struct MockFileSinkReceiver {
pub receiver: tokio::sync::mpsc::Receiver<SinkMessage>,
}

#[allow(dead_code)]
impl MockFileSinkReceiver {
pub async fn receive(&mut self) -> Option<Vec<u8>> {
match timeout(seconds(2), self.receiver.recv()).await {
Expand All @@ -25,15 +59,6 @@ impl MockFileSinkReceiver {
}
}

pub async fn get_all(&mut self) -> Vec<Vec<u8>> {
let mut buf = Vec::new();
while let Ok(SinkMessage::Data(on_write_tx, msg)) = self.receiver.try_recv() {
let _ = on_write_tx.send(Ok(()));
buf.push(msg);
}
buf
}

pub fn assert_no_messages(&mut self) {
let Err(TryRecvError::Empty) = self.receiver.try_recv() else {
panic!("receiver should have been empty")
Expand All @@ -56,7 +81,6 @@ impl MockFileSinkReceiver {
}
}

#[allow(dead_code)]
pub fn create_file_sink() -> (FileSinkClient, MockFileSinkReceiver) {
let (tx, rx) = tokio::sync::mpsc::channel(20);
(
Expand Down
6 changes: 6 additions & 0 deletions boost_manager/tests/integrations/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
mod common;

mod activator_tests;
mod purger_tests;
mod updater_tests;
mod watcher_tests;
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
mod common;
use boost_manager::purger;
use boost_manager::OnChainStatus;
use chrono::{DateTime, Duration, Utc};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,16 @@ const BOOSTED_HEX3_PUBKEY: &str = "11hd7HoicRgBPjBGcqcT2Y9hRQovdZeff5eKFMbCSuDYQ
const BOOSTED_HEX_CONFIG_PUBKEY: &str = "112QhnxqU8QZ3jUXpoRk51quuQVft9Pf5P5zzDDvLxj7Q9QqbMh7";

#[derive(Clone, Debug)]
#[allow(dead_code)]
pub struct MockTransaction {
signature: Signature,
activations: Vec<BoostedHexActivation>,
_activations: Vec<BoostedHexActivation>,
}

pub struct MockSolanaConnection {
submitted: Mutex<Vec<MockTransaction>>,
error: Option<String>,
}

#[derive(Clone, Debug)]
pub struct MockSignature {
pub signature: String,
}

#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error("not found")]
Expand Down Expand Up @@ -62,7 +56,7 @@ impl SolanaNetwork for MockSolanaConnection {
) -> Result<Self::Transaction, Self::Error> {
Ok(MockTransaction {
signature: Signature::new_unique(),
activations: batch.to_owned(),
_activations: batch.to_owned(),
})
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,48 +1,15 @@
mod common;
use crate::common::MockFileSinkReceiver;
use async_trait::async_trait;
use crate::common::{self, MockFileSinkReceiver, MockHexBoostingClient};
use boost_manager::watcher::{self, Watcher};
use chrono::{DateTime, Duration as ChronoDuration, Duration, Utc};
use futures_util::{stream, StreamExt as FuturesStreamExt};
use chrono::{Duration as ChronoDuration, Duration, Utc};
use helium_proto::BoostedHexInfoV1 as BoostedHexInfoProto;
use mobile_config::{
boosted_hex_info::{BoostedHexInfo, BoostedHexInfoStream},
client::{hex_boosting_client::HexBoostingInfoResolver, ClientError},
};
use mobile_config::boosted_hex_info::BoostedHexInfo;
use solana_sdk::pubkey::Pubkey;
use sqlx::PgPool;
use std::{num::NonZeroU32, str::FromStr};

const BOOST_HEX_PUBKEY: &str = "J9JiLTpjaShxL8eMvUs8txVw6TZ36E38SiJ89NxnMbLU";
const BOOST_CONFIG_PUBKEY: &str = "BZM1QTud72B2cpTW7PhEnFmRX7ZWzvY7DpPpNJJuDrWG";

#[derive(Debug, Clone)]
pub struct MockHexBoostingClient {
pub boosted_hexes: Vec<BoostedHexInfo>,
}

impl MockHexBoostingClient {
fn new(boosted_hexes: Vec<BoostedHexInfo>) -> Self {
Self { boosted_hexes }
}
}

#[async_trait]
impl HexBoostingInfoResolver for MockHexBoostingClient {
type Error = ClientError;

async fn stream_boosted_hexes_info(&mut self) -> Result<BoostedHexInfoStream, ClientError> {
Ok(stream::iter(self.boosted_hexes.clone()).boxed())
}

async fn stream_modified_boosted_hexes_info(
&mut self,
_timestamp: DateTime<Utc>,
) -> Result<BoostedHexInfoStream, ClientError> {
Ok(stream::iter(self.boosted_hexes.clone()).boxed())
}
}

#[sqlx::test]
async fn test_boosted_hex_updates_to_filestore(pool: PgPool) -> anyhow::Result<()> {
let (hex_update_client, mut hex_update) = common::create_file_sink();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ pub struct MockFileSinkReceiver {
pub receiver: tokio::sync::mpsc::Receiver<SinkMessage>,
}

#[allow(dead_code)]
impl MockFileSinkReceiver {
pub async fn receive(&mut self) -> Option<Vec<u8>> {
match timeout(seconds(2), self.receiver.recv()).await {
Expand Down Expand Up @@ -142,7 +141,6 @@ fn seconds(s: u64) -> std::time::Duration {
std::time::Duration::from_secs(s)
}

#[allow(dead_code)]
pub fn create_valid_beacon_report(
pubkey: &str,
received_timestamp: DateTime<Utc>,
Expand All @@ -165,7 +163,6 @@ pub fn create_valid_beacon_report(
)
}

#[allow(dead_code)]
pub fn create_valid_witness_report(
pubkey: &str,
received_timestamp: DateTime<Utc>,
Expand All @@ -186,7 +183,6 @@ pub fn create_valid_witness_report(
)
}

#[allow(dead_code)]
pub fn beacon_report_to_ingest_report(
report: IotBeaconReport,
received_timestamp: DateTime<Utc>,
Expand All @@ -197,7 +193,6 @@ pub fn beacon_report_to_ingest_report(
}
}

#[allow(dead_code)]
pub fn witness_report_to_ingest_report(
report: IotWitnessReport,
received_timestamp: DateTime<Utc>,
Expand All @@ -208,7 +203,6 @@ pub fn witness_report_to_ingest_report(
}
}

#[allow(dead_code)]
pub async fn inject_beacon_report(
pool: PgPool,
beacon: IotBeaconIngestReport,
Expand Down Expand Up @@ -237,7 +231,6 @@ pub async fn inject_beacon_report(
Ok(())
}

#[allow(dead_code)]
pub async fn inject_invalid_beacon_report(
pool: PgPool,
beacon: IotBeaconIngestReport,
Expand Down Expand Up @@ -266,7 +259,6 @@ pub async fn inject_invalid_beacon_report(
Ok(())
}

#[allow(dead_code)]
pub async fn inject_witness_report(
pool: PgPool,
witness: IotWitnessIngestReport,
Expand Down Expand Up @@ -295,7 +287,6 @@ pub async fn inject_witness_report(
Ok(())
}

#[allow(dead_code)]
pub async fn inject_entropy_report(pool: PgPool, ts: DateTime<Utc>) -> anyhow::Result<()> {
let data = REMOTE_ENTROPY.to_vec();
let id = hash(&data).as_bytes().to_vec();
Expand All @@ -305,7 +296,6 @@ pub async fn inject_entropy_report(pool: PgPool, ts: DateTime<Utc>) -> anyhow::R
Ok(())
}

#[allow(dead_code)]
pub async fn inject_last_beacon(
txn: &mut Transaction<'_, Postgres>,
gateway: PublicKeyBinary,
Expand All @@ -314,7 +304,6 @@ pub async fn inject_last_beacon(
LastBeaconReciprocity::update_last_timestamp(&mut *txn, &gateway, ts).await
}

#[allow(dead_code)]
pub async fn inject_last_witness(
txn: &mut Transaction<'_, Postgres>,
gateway: PublicKeyBinary,
Expand All @@ -323,7 +312,6 @@ pub async fn inject_last_witness(
LastWitness::update_last_timestamp(&mut *txn, &gateway, ts).await
}

#[allow(dead_code)]
pub fn valid_gateway() -> GatewayInfo {
GatewayInfo {
address: PublicKeyBinary::from_str(BEACONER1).unwrap(),
Expand All @@ -332,7 +320,6 @@ pub fn valid_gateway() -> GatewayInfo {
}
}

#[allow(dead_code)]
pub fn valid_gateway_stream() -> Vec<GatewayInfo> {
vec![
GatewayInfo {
Expand Down Expand Up @@ -413,7 +400,6 @@ pub fn valid_gateway_stream() -> Vec<GatewayInfo> {
]
}

#[allow(dead_code)]
pub fn valid_region_params() -> RegionParamsInfo {
let region_params =
beacon::RegionParams::from_bytes(ProtoRegion::Eu868.into(), 60, EU868_PARAMS, 0)
Expand Down
7 changes: 7 additions & 0 deletions iot_verifier/tests/integrations/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
mod common;

mod purger_tests;
mod rewarder_operations;
mod rewarder_oracles;
mod rewarder_poc_dc;
mod runner_tests;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
mod common;
use crate::common;
use chrono::{Duration as ChronoDuration, TimeZone, Utc};
use helium_crypto::PublicKeyBinary;
use helium_proto::services::poc_lora::{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
mod common;
use crate::common::MockFileSinkReceiver;
use crate::common::{self, MockFileSinkReceiver};
use chrono::{Duration as ChronoDuration, Utc};
use helium_proto::services::poc_lora::OperationalReward;
use iot_verifier::{reward_share, rewarder};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
mod common;
use crate::common::MockFileSinkReceiver;
use crate::common::{self, MockFileSinkReceiver};
use chrono::{Duration as ChronoDuration, Utc};
use helium_proto::services::poc_lora::UnallocatedReward;
use iot_verifier::{reward_share, rewarder};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
mod common;
use crate::common::MockFileSinkReceiver;
use crate::common::{self, MockFileSinkReceiver};
use chrono::{DateTime, Duration as ChronoDuration, Utc};
use helium_crypto::PublicKeyBinary;
use helium_proto::services::poc_lora::{GatewayReward, UnallocatedReward, UnallocatedRewardType};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
mod common;
use crate::common::{self, MockFileSinkReceiver};
use async_trait::async_trait;
use chrono::{DateTime, Duration as ChronoDuration, TimeZone, Utc};
use common::MockFileSinkReceiver;
use denylist::DenyList;
use futures_util::{stream, StreamExt as FuturesStreamExt};
use helium_crypto::PublicKeyBinary;
Expand Down
1 change: 0 additions & 1 deletion mobile_verifier/src/subscriber_location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ pub async fn aggregate_location_shares(
Ok(location_shares)
}

#[allow(dead_code)]
pub async fn clear_location_shares(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
timestamp: &DateTime<Utc>,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
mod common;
use crate::common;
use anyhow::Context;
use chrono::{DateTime, Duration, Utc};
use file_store::{
Expand Down
Loading

0 comments on commit 43d5a56

Please sign in to comment.