-
Notifications
You must be signed in to change notification settings - Fork 7
Add tests for add order file #1722
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
Conversation
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis change adds a new asynchronous method Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Function
participant AddOrderArgs
participant RainlangParser
participant TransactionArgs
Test->>AddOrderArgs: Create instance with test data
Test->>AddOrderArgs: Call compose_to_rainlang()
AddOrderArgs-->>Test: Return rainlang string or ComposeError
Test->>RainlangParser: try_parse_rainlang(rainlang string)
RainlangParser-->>Test: Return bytecode or ParseError
Test->>TransactionArgs: try_into_write_contract_parameters(addOrder2Call)
TransactionArgs-->>Test: Return contract parameters or Error
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
crates/common/src/add_order.rs
Outdated
} | ||
|
||
#[tokio::test] | ||
async fn test_add_order_call_try_into_write_contract_parameters() { |
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.
I'm a bit confused - is this testing any logic from this module?
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.
i have added this test because we cannot mock the ledger logic. this was the function that was called inside the execute
function so i have added this to test out to flow inside that function
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.
maybe it'd be better to take that bit out of execute
, put it in a new function and test that? otherwise the logic in execute
can diverge from the test and the test wouldn't catch any potential issues
crates/common/src/add_order.rs
Outdated
@@ -974,4 +978,225 @@ _ _: 16 52; | |||
.await | |||
.expect_err("expected to fail but resolved"); | |||
} | |||
|
|||
#[tokio::test] | |||
async fn test_try_parse_rainlang() { |
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.
we should probably also cover unhappy path of try_parse_rainlang
crates/common/src/add_order.rs
Outdated
} | ||
|
||
#[tokio::test] | ||
async fn test_get_add_order_calldata_invalid_rcp_url() { |
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.
typo
async fn test_get_add_order_calldata_invalid_rcp_url() { | |
async fn test_get_add_order_calldata_invalid_rpc_url() { |
crates/common/src/add_order.rs
Outdated
} | ||
|
||
#[tokio::test] | ||
async fn test_add_order_call_try_into_write_contract_parameters() { |
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.
maybe it'd be better to take that bit out of execute
, put it in a new function and test that? otherwise the logic in execute
can diverge from the test and the test wouldn't catch any potential issues
@0xgleb to test out |
Motivation
See issue: #1623
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
[ ] included screenshots (if this involves a front-end change)fix #1623
Summary by CodeRabbit