-
Notifications
You must be signed in to change notification settings - Fork 89
test(locked_outpoints): add unit tests for ChangeSet merge and is_empty #436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -417,3 +417,116 @@ impl From<locked_outpoints::ChangeSet> for ChangeSet { | |||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| #[cfg(test)] | ||||||
| mod test { | ||||||
| use super::*; | ||||||
| use alloc::string::ToString; | ||||||
| use bdk_chain::Merge; | ||||||
| use bitcoin::Network; | ||||||
|
|
||||||
| /// Helper to get public descriptors via a temporary wallet. | ||||||
| fn test_descriptors() -> ( | ||||||
| Descriptor<DescriptorPublicKey>, | ||||||
| Descriptor<DescriptorPublicKey>, | ||||||
| ) { | ||||||
| let (desc, change_desc) = crate::test_utils::get_test_wpkh_and_change_desc(); | ||||||
| let wallet = crate::Wallet::create(desc.to_string(), change_desc.to_string()) | ||||||
| .network(Network::Regtest) | ||||||
| .create_wallet_no_persist() | ||||||
| .unwrap(); | ||||||
| ( | ||||||
| wallet | ||||||
| .public_descriptor(crate::KeychainKind::External) | ||||||
| .clone(), | ||||||
| wallet | ||||||
| .public_descriptor(crate::KeychainKind::Internal) | ||||||
| .clone(), | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_default_is_empty() { | ||||||
| let cs = ChangeSet::default(); | ||||||
| assert!(cs.is_empty()); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_is_empty_with_descriptor() { | ||||||
| let (desc, _) = test_descriptors(); | ||||||
| let cs = ChangeSet { | ||||||
| descriptor: Some(desc), | ||||||
| ..Default::default() | ||||||
| }; | ||||||
| assert!(!cs.is_empty()); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_is_empty_with_network() { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| let cs = ChangeSet { | ||||||
| network: Some(Network::Regtest), | ||||||
| ..Default::default() | ||||||
| }; | ||||||
| assert!(!cs.is_empty()); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_merge_sets_descriptor() { | ||||||
| let (desc, _) = test_descriptors(); | ||||||
| let mut cs = ChangeSet::default(); | ||||||
| let other = ChangeSet { | ||||||
| descriptor: Some(desc.clone()), | ||||||
| ..Default::default() | ||||||
| }; | ||||||
| cs.merge(other); | ||||||
| assert_eq!(cs.descriptor, Some(desc)); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_merge_sets_network() { | ||||||
| let mut cs = ChangeSet::default(); | ||||||
| let other = ChangeSet { | ||||||
| network: Some(Network::Regtest), | ||||||
| ..Default::default() | ||||||
| }; | ||||||
| cs.merge(other); | ||||||
| assert_eq!(cs.network, Some(Network::Regtest)); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_merge_same_descriptor_is_ok() { | ||||||
| let (desc, _) = test_descriptors(); | ||||||
| let mut cs = ChangeSet { | ||||||
| descriptor: Some(desc.clone()), | ||||||
| ..Default::default() | ||||||
| }; | ||||||
| let other = ChangeSet { | ||||||
| descriptor: Some(desc.clone()), | ||||||
| ..Default::default() | ||||||
| }; | ||||||
| cs.merge(other); | ||||||
| assert_eq!(cs.descriptor, Some(desc)); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_merge_empty_into_non_empty() { | ||||||
| let (desc, change_desc) = test_descriptors(); | ||||||
| let mut cs = ChangeSet { | ||||||
| descriptor: Some(desc), | ||||||
| change_descriptor: Some(change_desc), | ||||||
| network: Some(Network::Regtest), | ||||||
| ..Default::default() | ||||||
| }; | ||||||
| let snapshot = cs.clone(); | ||||||
| cs.merge(ChangeSet::default()); | ||||||
| assert_eq!(cs, snapshot); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_from_local_chain() { | ||||||
| let chain_cs = local_chain::ChangeSet::default(); | ||||||
| let cs = ChangeSet::from(chain_cs); | ||||||
| assert!(cs.descriptor.is_none()); | ||||||
| assert!(cs.network.is_none()); | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,3 +24,81 @@ impl Merge for ChangeSet { | |||||
| self.outpoints.is_empty() | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| #[cfg(test)] | ||||||
| mod test { | ||||||
| use super::*; | ||||||
| use bdk_chain::Merge; | ||||||
| use bitcoin::{hashes::Hash, OutPoint, Txid}; | ||||||
|
|
||||||
| /// Helper to create an `OutPoint` from an index byte. | ||||||
| fn outpoint(vout: u32) -> OutPoint { | ||||||
| OutPoint { | ||||||
| txid: Txid::from_byte_array([vout as u8; 32]), | ||||||
| vout, | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_is_empty_default() { | ||||||
| let cs = ChangeSet::default(); | ||||||
| assert!(cs.is_empty()); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_is_empty_with_entries() { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I saw the test name and read through what the test tests, I was confused. @ValuedMammal made a suggestion in src/wallet/changeset.rs to update the test names so they reflect the expected outcome rather than just the method being called. I think that applies to this too. |
||||||
| let cs = ChangeSet { | ||||||
| outpoints: [(outpoint(0), true)].into(), | ||||||
| }; | ||||||
| assert!(!cs.is_empty()); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_merge_into_empty() { | ||||||
| let mut cs = ChangeSet::default(); | ||||||
| let other = ChangeSet { | ||||||
| outpoints: [(outpoint(1), true)].into(), | ||||||
| }; | ||||||
| cs.merge(other); | ||||||
| assert_eq!(cs.outpoints.len(), 1); | ||||||
| assert!(cs.outpoints[&outpoint(1)]); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_merge_empty_into_non_empty() { | ||||||
| let mut cs = ChangeSet { | ||||||
| outpoints: [(outpoint(0), true)].into(), | ||||||
| }; | ||||||
| let snapshot = cs.clone(); | ||||||
| cs.merge(ChangeSet::default()); | ||||||
| assert_eq!(cs, snapshot); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_merge_disjoint() { | ||||||
| let mut cs = ChangeSet { | ||||||
| outpoints: [(outpoint(0), true)].into(), | ||||||
| }; | ||||||
| let other = ChangeSet { | ||||||
| outpoints: [(outpoint(1), false)].into(), | ||||||
| }; | ||||||
| cs.merge(other); | ||||||
| assert_eq!(cs.outpoints.len(), 2); | ||||||
| assert!(cs.outpoints[&outpoint(0)]); | ||||||
| assert!(!cs.outpoints[&outpoint(1)]); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_merge_overwrites_duplicate() { | ||||||
| let op = outpoint(0); | ||||||
| let mut cs = ChangeSet { | ||||||
| outpoints: [(op, true)].into(), | ||||||
| }; | ||||||
| let other = ChangeSet { | ||||||
| outpoints: [(op, false)].into(), | ||||||
| }; | ||||||
| cs.merge(other); | ||||||
| assert_eq!(cs.outpoints.len(), 1); | ||||||
| assert!(!cs.outpoints[&op], "other's value should overwrite self"); | ||||||
| } | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be more clear to the reader if the test name reflected the expectation.