Skip to content

Conversation

@Niederb
Copy link
Contributor

@Niederb Niederb commented Jun 11, 2024

No description provided.

@Niederb Niederb self-assigned this Jun 11, 2024
@Niederb Niederb linked an issue Jun 11, 2024 that may be closed by this pull request
@Niederb Niederb force-pushed the tn/balance_transfer_keep_alive branch from 02e1e61 to f9f4a4b Compare July 8, 2024 07:18
@Niederb Niederb marked this pull request as ready for review July 8, 2024 08:15
@Niederb Niederb added F8-newfeature Introduces a new feature E1-breaksnothing labels Jul 8, 2024
@Niederb Niederb requested a review from masapr July 8, 2024 08:29
let mut api = Api::<AssetRuntimeConfig, _>::new(client).await.unwrap();

let _ed = api.get_existential_deposit().await.unwrap();
let ed = api.get_existential_deposit().await.unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the focus of this test is sending funds, therefore I would make a new test-file with a fitting name. Also I would test a successfull transaction with balance_transfer_keep_alive.

I would create 2 new tests:

  1. "send_funds_test.rs" that tests a successfull balance_transfer_keep_alive transaction
  2. "send_funds_with_death_test.rs" for the test case here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with adding a successful transaction for balance_transfer_keep_alive but I would keep it all in the same file it is now. The reason is that so far the testing examples are more organized by pallet/subsystem (frame_system_tests.rs, pallet_contract_tests.rs, chain_tests.rs, ....) and not by a specific test case.

Copy link
Collaborator

@masapr masapr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@Niederb Niederb merged commit 59b0f4d into master Jul 10, 2024
@Niederb Niederb deleted the tn/balance_transfer_keep_alive branch December 9, 2025 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

E1-breaksnothing F8-newfeature Introduces a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

implement balance_transfer_keep_alive

3 participants