Skip to content

How to write unit-tests for substrate#3

Open
Niraj-Kamdar wants to merge 1 commit intoframe-system-custom-configfrom
basic-tests
Open

How to write unit-tests for substrate#3
Niraj-Kamdar wants to merge 1 commit intoframe-system-custom-configfrom
basic-tests

Conversation

@Niraj-Kamdar
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey @Niraj-Kamdar - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟡 Testing: 3 issues found
  • 🟢 Complexity: all looks good
  • 🟢 Docstrings: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

Comment thread runtime/src/lib.rs
pub type MinimumBalance = <Runtime as pallet_balances::Config>::ExistentialDeposit;
}

#[cfg(test)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (testing): Consider adding edge case tests for balance transfers.

It's great to see tests for normal balance transfer scenarios. However, adding tests for edge cases, such as transferring balance when the sender does not have enough funds or transferring a zero balance, would enhance the robustness of the test suite.

Comment thread runtime/src/lib.rs
pub type MinimumBalance = <Runtime as pallet_balances::Config>::ExistentialDeposit;
}

#[cfg(test)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (testing): Add tests for error conditions in balance transfers.

Testing successful balance transfers is crucial, but it's equally important to verify the system's behavior under error conditions. For instance, tests that verify the system's response when attempting to transfer more than the account's balance or to an invalid account would be valuable.

Comment thread runtime/src/lib.rs
pub type MinimumBalance = <Runtime as pallet_balances::Config>::ExistentialDeposit;
}

#[cfg(test)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (testing): Include tests for block number increments.

The current test suite checks the block number's set and get functionality but does not cover scenarios where the block number increments over time or through specific actions. Including such tests could provide more insights into the system's behavior over time.

@krisbitney
Copy link
Copy Markdown

This is really interesting and helpful. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants