refactor: program test reduce forester utils dep#1867
Conversation
WalkthroughThe changes refactor the test utilities for registering a "forester" account by moving the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Code
participant Utils as register_test_forester (Utility)
participant RPC as RPC Interface
participant Chain as Solana Chain
Test->>Utils: register_test_forester(governance_authority, forester_authority, config)
Utils->>RPC: send registration instruction
RPC->>Chain: submit transaction
Chain-->>RPC: transaction result
Utils->>RPC: fetch forester account data
RPC-->>Utils: return account data
Utils->>Test: assert registration result
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
program-tests/utils/src/e2e_test_env.rs (1)
81-81: Import can be shortened to avoid the doubleregister_test_foresterrepetitionBringing the function in with the full module → function path
use light_program_test::utils::register_test_forester::register_test_forester;leads to the somewhat verbose call‐site pattern
register_test_forester(/* … */).If no other symbols from
register_test_foresterare needed, you could import it with an alias and keep the intent clear:-use light_program_test::utils::register_test_forester::register_test_forester; +use light_program_test::utils::register_test_forester::register_test_forester as register_forester;and later
- register_test_forester( + register_forester(Purely cosmetic, but reduces the cognitive load when scanning the file.
sdk-libs/program-test/src/utils/create_account.rs (1)
13-13: Consider security implications ofinsecure_clone().The use of
insecure_clone()may be acceptable in test utilities but should be carefully considered. Ensure this function is only used in test contexts where security is not a concern.sdk-libs/program-test/src/utils/tree_accounts.rs (1)
13-25: Consider reordering parameters to match struct field order.The parameter order in the constructor doesn't match the struct field order, which could be confusing for maintainability.
Apply this diff to align parameter order with struct fields:
pub fn new( merkle_tree: Pubkey, queue: Pubkey, - tree_type: TreeType, is_rolledover: bool, + tree_type: TreeType, ) -> Self {sdk-libs/program-test/src/utils/register_test_forester.rs (2)
31-42: Consider adding a small delay before assertion.The function immediately asserts the forester registration after sending the transaction. In blockchain environments, there might be a small delay before the transaction is confirmed and the account state is updated.
Consider adding a small delay or retry mechanism:
) .await?; +// Small delay to ensure transaction is processed +tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; assert_registered_forester(
34-39: Consider explicit field initialization over Default::default().Using
..Default::default()might hide important fields that should be explicitly set in tests. Consider initializing all fields explicitly for better test clarity.ForesterPda { authority: *forester_authority, config, active_weight: 1, - ..Default::default() + // Add explicit initialization for remaining fields + epoch: 0, + total_weight: 0, + // ... other fields }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
forester-utils/src/registry.rs(1 hunks)forester/tests/legacy/batched_address_test.rs(1 hunks)program-tests/utils/src/e2e_test_env.rs(1 hunks)program-tests/utils/src/lib.rs(1 hunks)sdk-libs/program-test/src/accounts/address_tree.rs(1 hunks)sdk-libs/program-test/src/accounts/address_tree_v2.rs(1 hunks)sdk-libs/program-test/src/accounts/initialize.rs(2 hunks)sdk-libs/program-test/src/accounts/state_tree.rs(1 hunks)sdk-libs/program-test/src/accounts/state_tree_v2.rs(1 hunks)sdk-libs/program-test/src/utils/create_account.rs(1 hunks)sdk-libs/program-test/src/utils/mod.rs(1 hunks)sdk-libs/program-test/src/utils/register_test_forester.rs(1 hunks)sdk-libs/program-test/src/utils/tree_accounts.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
sdk-libs/program-test/src/utils/mod.rs (3)
sdk-libs/macros/tests/hasher.rs (1)
create_account(116-124)sdk-libs/program-test/src/utils/register_test_forester.rs (1)
register_test_forester(13-42)sdk-libs/program-test/src/utils/setup_light_programs.rs (1)
setup_light_programs(26-114)
program-tests/utils/src/e2e_test_env.rs (1)
sdk-libs/program-test/src/utils/register_test_forester.rs (1)
register_test_forester(13-42)
forester/tests/legacy/batched_address_test.rs (1)
forester-utils/src/registry.rs (1)
update_test_forester(31-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-test"])
- GitHub Check: Forester e2e test
- GitHub Check: system-programs (counter-test, ["cargo test-sbf -p counter"])
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (token-escrow-test, [ "cargo test-sbf -p token-escrow" ])
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test"])
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: Test program-libs-fast
- GitHub Check: Test program-libs-slow
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test sdk-libs
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: cli-v2
- GitHub Check: stateless-js-v2
- GitHub Check: stateless-js-v1
- GitHub Check: cli-v1
- GitHub Check: lint
🔇 Additional comments (13)
sdk-libs/program-test/src/accounts/address_tree_v2.rs (1)
1-1: Same visibility caveat as aboveMake sure
create_account_instructionis publicly reachable fromcrate::utils::create_account; otherwise this new import will break.sdk-libs/program-test/src/accounts/state_tree_v2.rs (1)
2-2: Confirm utils subtree is exportedThis file now depends on the same symbol. Ensure the
utilsmodule hierarchy doesn’t accidentally hide the function behindpub(crate).sdk-libs/program-test/src/accounts/state_tree.rs (1)
6-6: Visibility check againIdentical comment: verify
create_account_instructionis publicly exported so external callers (e.g. integration tests) compile.program-tests/utils/src/lib.rs (1)
49-53: LGTM! Clean import path refactoring.The import path change from
forester_utils::registrytolight_program_test::utils::register_test_foresteris consistent with the PR objective to reduce forester-utils dependencies. The public API remains unchanged while the implementation is now sourced locally.forester-utils/src/registry.rs (1)
14-14: LGTM! Appropriate cleanup of test utilities.The removal of the
register_test_foresterfunction and its associated import aligns with the PR objective to move test utilities out of forester-utils and into the program-test crate. This improves separation of concerns between production and test code.sdk-libs/program-test/src/utils/mod.rs (1)
2-6: LGTM! Well-organized utility module expansion.The addition of
create_account,register_test_forester, andtree_accountsmodules appropriately expands the local utility capabilities and supports the dependency reduction objective. The module names are descriptive and the organization is logical.sdk-libs/program-test/src/accounts/initialize.rs (2)
2-4: LGTM! Consistent import refactoring.The import changes properly utilize local utilities (
register_test_foresterandTreeAccounts) instead of external forester-utils dependencies, maintaining consistency with the PR objective to reduce external dependencies.
235-238: LGTM! Proper type conversion handling.The conversion from local
TreeAccountstoforester_utils::forester_epoch::TreeAccountsusing theto_forester_utils()method maintains compatibility while allowing for local type definitions. This is a clean approach to handling type boundaries between modules.sdk-libs/program-test/src/utils/tree_accounts.rs (2)
4-10: LGTM: Well-structured struct with appropriate traits.The struct design is clean with proper field visibility and derives appropriate traits for a data structure used in testing scenarios.
27-35: Constructor signature verifiedThe external
TreeAccounts::new(merkle_tree: Pubkey, queue: Pubkey, tree_type: TreeType, is_rolledover: bool)exactly matches the arguments passed into_forester_utils(). No changes required.sdk-libs/program-test/src/utils/register_test_forester.rs (3)
12-18: LGTM: Well-documented function signature.The function signature is clear and the documentation accurately describes the function's purpose.
44-58: LGTM: Proper error handling and account verification.The assertion function correctly fetches the account data and compares it with the expected values, providing meaningful error messages on failure.
19-24: No action needed for duplicate governance authority usageThe first argument to
create_register_forester_instructionis the fee payer, and throughout the codebase—includingxtask/src/new_deployment.rs—we intentionally pass the samegovernance_authority.pubkey()for both fee payer and governance authority. This duplication is by design and does not require changes.Likely an incorrect or invalid review comment.
| }; | ||
| use anchor_lang::InstructionData; | ||
| use forester_utils::instructions::create_account::create_account_instruction; | ||
| use crate::utils::create_account::create_account_instruction; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Import path looks correct—just confirm the module is publicly re-exported
The switch to crate::utils::create_account::create_account_instruction will compile only if mod utils; is declared at the crate root and pub mod create_account; (or pub use) exposes the function. Double-check that visibility is set; otherwise downstream crates/tests won’t see it.
Run a quick grep to verify the public re-export:
🏁 Script executed:
#!/bin/bash
# Expect at least one `pub` re-export of `create_account_instruction`
rg --line-number "pub .*create_account_instruction" | headLength of output: 327
🏁 Script executed:
#!/bin/bash
# Verify that `mod utils;` is declared at the crate root of program-test
echo "== Checking for `mod utils;` in sdk-libs/program-test/src/lib.rs =="
rg --line-number "^mod utils;" -n sdk-libs/program-test/src/lib.rs || echo "— mod utils; not found in lib.rs"
# If no lib.rs, check main.rs
echo "== Checking for `mod utils;` in sdk-libs/program-test/src/main.rs =="
rg --line-number "^mod utils;" -n sdk-libs/program-test/src/main.rs || echo "— mod utils; not found in main.rs"
# Verify that `pub mod create_account;` is exposed in utils
echo "== Checking for `pub mod create_account;` in sdk-libs/program-test/src/utils/mod.rs =="
rg --line-number "^pub mod create_account;" -n sdk-libs/program-test/src/utils/mod.rs || echo "— pub mod create_account; not found in utils/mod.rs"Length of output: 1190
Add and expose the utils module in the crate root
It looks like you’ve got pub mod create_account; in
sdk-libs/program-test/src/utils/mod.rs and a pub fn create_account_instruction,
but there’s no mod utils; in your crate root, so crate::utils::… won’t resolve.
Please update sdk-libs/program-test/src/lib.rs to include:
mod accounts;
+mod utils;
+// If you need this function accessible to downstream crates:
+pub use utils::create_account::create_account_instruction;This ensures:
crate::utils::create_account::create_account_instructioncompiles internally- External consumers can import
create_account_instructiondirectly, if required.
🤖 Prompt for AI Agents
In sdk-libs/program-test/src/lib.rs, add the line `pub mod utils;` to declare
and expose the utils module at the crate root. This will allow
`crate::utils::create_account::create_account_instruction` to resolve correctly
within the crate and enable external consumers to import
`create_account_instruction` directly if needed.
| use forester_utils::{ | ||
| registry::{register_test_forester, update_test_forester}, | ||
| registry::update_test_forester, | ||
| rpc_pool::SolanaRpcPoolBuilder, | ||
| }; |
There was a problem hiding this comment.
register_test_forester no longer imported – will not compile
You removed the old import but calls at lines 87-93 still use the function. Add the new path import from light_program_test (or qualify the call).
-use forester_utils::{
- registry::update_test_forester,
- rpc_pool::SolanaRpcPoolBuilder,
-};
+use forester_utils::{
+ registry::update_test_forester,
+ rpc_pool::SolanaRpcPoolBuilder,
+};
+use light_program_test::utils::register_test_forester;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use forester_utils::{ | |
| registry::{register_test_forester, update_test_forester}, | |
| registry::update_test_forester, | |
| rpc_pool::SolanaRpcPoolBuilder, | |
| }; | |
| use forester_utils::{ | |
| registry::update_test_forester, | |
| rpc_pool::SolanaRpcPoolBuilder, | |
| }; | |
| use light_program_test::utils::register_test_forester; |
🤖 Prompt for AI Agents
In forester/tests/legacy/batched_address_test.rs around lines 4 to 7, the
function register_test_forester is no longer imported but is still called at
lines 87-93. To fix this, add the correct import for register_test_forester from
the light_program_test module at the top of the file or fully qualify the calls
to register_test_forester where used. This will ensure the code compiles
successfully.
| @@ -0,0 +1,17 @@ | |||
| use account_compression::processor::initialize_address_merkle_tree::Pubkey; | |||
There was a problem hiding this comment.
Fix the incorrect Pubkey import.
The import path for Pubkey from account_compression::processor::initialize_address_merkle_tree::Pubkey is incorrect. Pubkey should be imported from the standard Solana SDK location.
Apply this diff to fix the import:
-use account_compression::processor::initialize_address_merkle_tree::Pubkey;
+use solana_sdk::pubkey::Pubkey;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use account_compression::processor::initialize_address_merkle_tree::Pubkey; | |
| -use account_compression::processor::initialize_address_merkle_tree::Pubkey; | |
| +use solana_sdk::pubkey::Pubkey; |
🤖 Prompt for AI Agents
In sdk-libs/program-test/src/utils/create_account.rs at line 1, the import path
for Pubkey is incorrect. Replace the current import from
account_compression::processor::initialize_address_merkle_tree::Pubkey with the
correct import from the standard Solana SDK, typically
solana_program::pubkey::Pubkey, to ensure the correct Pubkey type is used.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
program-tests/utils/src/setup_forester.rs (1)
50-63: Consider making tree accounts configurable.The function creates hardcoded tree accounts using test keypairs. This might not be flexible enough for all test scenarios that require different tree configurations.
Consider adding an optional parameter to allow custom tree accounts:
pub async fn setup_forester_and_advance_to_epoch<R: Rpc + TestRpc>( context: &mut R, protocol_config: &ProtocolConfig, + custom_tree_accounts: Option<Vec<TreeAccounts>>, ) -> Result<Epoch, RpcError> {Then use the custom accounts if provided, or fall back to the default ones:
- // Create tree accounts for the epoch using test keypairs - let tree_accounts = vec![ - TreeAccounts::new( - test_keypairs.state_merkle_tree.pubkey(), - test_keypairs.nullifier_queue.pubkey(), - TreeType::StateV1, - false, - ), - TreeAccounts::new( - test_keypairs.address_merkle_tree.pubkey(), - test_keypairs.address_merkle_tree_queue.pubkey(), - TreeType::AddressV1, - false, - ), - ]; + let tree_accounts = custom_tree_accounts.unwrap_or_else(|| vec![ + TreeAccounts::new( + test_keypairs.state_merkle_tree.pubkey(), + test_keypairs.nullifier_queue.pubkey(), + TreeType::StateV1, + false, + ), + TreeAccounts::new( + test_keypairs.address_merkle_tree.pubkey(), + test_keypairs.address_merkle_tree_queue.pubkey(), + TreeType::AddressV1, + false, + ), + ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
program-tests/registry-test/tests/tests.rs(9 hunks)program-tests/system-test/tests/test.rs(4 hunks)program-tests/utils/src/e2e_test_env.rs(2 hunks)program-tests/utils/src/lib.rs(2 hunks)program-tests/utils/src/setup_forester.rs(1 hunks)sdk-libs/program-test/Cargo.toml(1 hunks)sdk-libs/program-test/src/accounts/initialize.rs(0 hunks)sdk-libs/program-test/src/accounts/test_accounts.rs(0 hunks)sdk-libs/program-test/src/program_test/config.rs(0 hunks)sdk-libs/program-test/src/utils/tree_accounts.rs(1 hunks)xtask/src/new_deployment.rs(0 hunks)
💤 Files with no reviewable changes (4)
- sdk-libs/program-test/src/program_test/config.rs
- sdk-libs/program-test/src/accounts/test_accounts.rs
- sdk-libs/program-test/src/accounts/initialize.rs
- xtask/src/new_deployment.rs
✅ Files skipped from review due to trivial changes (1)
- sdk-libs/program-test/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (3)
- program-tests/utils/src/lib.rs
- sdk-libs/program-test/src/utils/tree_accounts.rs
- program-tests/utils/src/e2e_test_env.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
program-tests/registry-test/tests/tests.rs (1)
program-tests/utils/src/setup_forester.rs (1)
setup_forester_and_advance_to_epoch(22-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: lint
- GitHub Check: cli-v1
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test program-libs-fast
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: Test sdk-libs
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: Test program-libs-slow
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test"])
- GitHub Check: stateless-js-v1
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: stateless-js-v2
- GitHub Check: Forester e2e test
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-test"])
- GitHub Check: system-programs (token-escrow-test, [ "cargo test-sbf -p token-escrow" ])
- GitHub Check: cli-v2
- GitHub Check: system-programs (counter-test, ["cargo test-sbf -p counter"])
🔇 Additional comments (13)
program-tests/system-test/tests/test.rs (3)
37-37: LGTM - Clean import of the new utility function.The import properly brings in the new forester setup utility function that consolidates the previously scattered forester initialization logic.
677-680: LGTM - Proper usage of the new forester setup utility.The function is correctly awaited and the returned
forester_epochis used to access epoch information on subsequent lines (717, 721). This replaces the previous implicit forester setup logic.
849-852: LGTM - Consistent usage of the forester setup utility.The function usage is consistent with the other test function, properly awaiting the result and using the returned epoch information.
program-tests/utils/src/setup_forester.rs (2)
12-25: LGTM - Well-documented function with clear purpose.The documentation clearly explains the function's purpose and parameters, making it easy to understand how to use this utility function.
22-85: LGTM - Well-structured forester setup function.The function effectively consolidates the forester setup process into a single reusable utility. The logical flow is clear: register forester → register epoch → advance to active phase → create tree accounts → add trees → finalize registration.
program-tests/registry-test/tests/tests.rs (8)
77-77: LGTM: Import addition is properly placedThe import for
setup_forester_and_advance_to_epochis correctly added to the imports section and aligns with the refactoring objective to centralize forester setup logic.
556-558: LGTM: Consistent setup pattern implementedThe helper function call follows the established pattern with proper parameter passing and error handling. The protocol config is correctly cloned from the RPC context.
639-641: LGTM: Consistent setup pattern implementedThe helper function call follows the established pattern with proper parameter passing and error handling. The protocol config is correctly cloned from the RPC context.
1031-1033: LGTM: Consistent setup pattern implementedThe helper function call follows the established pattern with proper parameter passing and error handling. The protocol config is correctly cloned from the RPC context.
1426-1428: LGTM: Consistent setup pattern implementedThe helper function call follows the established pattern with proper parameter passing and error handling. The protocol config is correctly cloned from the RPC context.
1699-1701: LGTM: Consistent setup pattern implementedThe helper function call follows the established pattern with proper parameter passing and error handling. The protocol config is correctly cloned from the RPC context.
1894-1896: LGTM: Consistent setup pattern implementedThe helper function call follows the established pattern with proper parameter passing and error handling. The protocol config is correctly cloned from the RPC context.
2074-2076: LGTM: Consistent setup pattern implementedThe helper function call follows the established pattern with proper parameter passing and error handling. The protocol config is correctly cloned from the RPC context.
| use forester::run_pipeline; | ||
| use forester_utils::{ | ||
| registry::{register_test_forester, update_test_forester}, | ||
| registry::update_test_forester, |
There was a problem hiding this comment.
wdyt about moving update_test_forester too?
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
program-tests/utils/src/update_test_forester.rs (3)
11-17: Add function documentation.The function lacks documentation explaining its purpose, parameters, and behavior. This would improve maintainability and help other developers understand the API.
+/// Updates a forester PDA account with new configuration and optionally a new authority. +/// +/// # Arguments +/// * `rpc` - RPC client for blockchain operations +/// * `forester_authority` - Current authority keypair for the forester +/// * `derivation_key` - Key used to derive the forester PDA +/// * `new_forester_authority` - Optional new authority to transfer ownership to +/// * `config` - New configuration to apply to the forester +/// +/// # Returns +/// Result indicating success or RPC error pub async fn update_test_forester<R: Rpc>(
18-21: Consider handling missing account more gracefully.The
unwrap()on line 21 will panic if the forester account doesn't exist. While this might be acceptable for test utilities, consider providing a more descriptive error message.- let mut pre_account_state = rpc - .get_anchor_account::<ForesterPda>(&get_forester_pda(derivation_key).0) - .await? - .unwrap(); + let mut pre_account_state = rpc + .get_anchor_account::<ForesterPda>(&get_forester_pda(derivation_key).0) + .await? + .ok_or_else(|| RpcError::AssertRpcError( + "Forester account not found. Ensure the forester is registered before updating.".to_string() + ))?;
22-31: Clarify variable naming to avoid confusion.The variable
new_authorityin the tuple destructuring (line 27) shadows the parameternew_forester_authority, which could be confusing. Consider using a more descriptive name.let (signers, new_forester_authority) = if let Some(new_authority) = new_forester_authority { pre_account_state.authority = new_authority.pubkey(); ( vec![forester_authority, &new_authority], - Some(new_authority.pubkey()), + Some(new_authority.pubkey()), ) } else { (vec![forester_authority], None) };Actually, the variable naming is acceptable since it's scoped within the if-let. The code is correct as written.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
program-tests/utils/src/update_test_forester.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: stateless-js-v1
- GitHub Check: lint
- GitHub Check: Test program-libs-slow
- GitHub Check: Test sdk-libs
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-fast
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: cli-v1
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-test"])
- GitHub Check: system-programs (counter-test, ["cargo test-sbf -p counter"])
- GitHub Check: system-programs (token-escrow-test, [ "cargo test-sbf -p token-escrow" ])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: cli-v2
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: Forester e2e test
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: stateless-js-v2
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
🔇 Additional comments (2)
program-tests/utils/src/update_test_forester.rs (2)
32-44: LGTM! Transaction creation and verification logic is sound.The instruction creation, transaction sending, and post-update verification follow proper patterns. The approach of updating the expected state and then asserting against it is appropriate for test utilities.
46-60: LGTM! Helper function provides clear error reporting.The assertion function correctly fetches the account data and provides meaningful error messages when the actual state doesn't match expectations. The error format clearly shows both expected and actual values for debugging.
ananas-block
force-pushed
the
jorrit/refactor-program-test-reduce-forester-utils-dep
branch
from
c0c78cf to
e7560c5
Compare
ananas-block
deleted the
jorrit/refactor-program-test-reduce-forester-utils-dep
branch
2 participants
Summary by CodeRabbit
New Features
Refactor
Chores
.gitignoreto exclude directories named~at any depth.Bug Fixes