Conversation
Add: auction tests
WalkthroughUnifies chain initialization and context creation to use a single UTC timestamp in Go, adds Rust integration test coverage for current auction basket time-flow, updates the CI workflow to use --locked for cargo commands, and removes Cargo.lock from .gitignore so the lockfile is tracked. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Runner
participant App as TestEnv Setup
participant Chain as Chain
Test->>App: Initialize TestEnv
App->>App: now = time.Now().UTC()
App->>Chain: InitChain(Time: now)
App->>Chain: NewUncachedContext(Header.Time: now)
sequenceDiagram
participant T as Auction Test
participant App as InjectiveTestApp
participant Auction as Auction Module
T->>Auction: query_current_auction_basket()
Auction-->>T: round=0, closing_time>block_time
T->>App: advance_time(+1s)
T->>Auction: query_current_auction_basket()
Auction-->>T: round unchanged, closing_time unchanged
T->>App: advance_time(to closing_time - block_time + 1s)
T->>Auction: query_current_auction_basket()
Auction-->>T: round increased, closing_time increased
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/injective-test-tube/libinjectivetesttube/testenv/setup.go (1)
156-156: Nit: renamenowtogenesisTimefor intent clarityMinor readability improvement: the variable represents the chain’s genesis timestamp.
- now := time.Now().UTC() + genesisTime := time.Now().UTC() @@ - Time: now, + Time: genesisTime, @@ - ctx := appInstance.NewUncachedContext(false, cmtproto.Header{Height: 0, ChainID: "injective-777", Time:now}) + ctx := appInstance.NewUncachedContext(false, cmtproto.Header{Height: 0, ChainID: "injective-777", Time: genesisTime})Also applies to: 165-165, 169-169
packages/injective-test-tube/src/module/auction.rs (2)
106-110: Nit: remove redundant cast
block_time_secis already a u64; theas u64is unnecessary.- closing_time > block_time_sec as u64, + closing_time > block_time_sec,
128-142: Avoid overshooting: compute delta from the current block time (after the +1s bump)After
increase_time(1), you addclosing_time - block_time_sec + 1using the originalblock_time_sec, which overshoots by one extra second. Compute the delta relative to the current time to make the intent explicit and robust. Also safer to use saturating_sub to prevent underflow in case of unexpected clock drift.- app.increase_time(closing_time - block_time_sec + 1); + let block_time_sec_after_increase = app.get_block_time_seconds() as u64; + let delta = closing_time.saturating_sub(block_time_sec_after_increase) + 1; + app.increase_time(delta);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/injective-test-tube/libinjectivetesttube/testenv/setup.go(1 hunks)packages/injective-test-tube/src/module/auction.rs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/injective-test-tube/src/module/auction.rs (1)
packages/injective-test-tube/src/runner/app.rs (1)
app(301-302)
⏰ 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). (4)
- GitHub Check: Lints
- GitHub Check: Test Suite
- GitHub Check: Test Suite
- GitHub Check: Lints
🔇 Additional comments (4)
packages/injective-test-tube/libinjectivetesttube/testenv/setup.go (1)
156-156: Good call: single genesis timestamp for InitChain and ContextUsing a single UTC timestamp eliminates subtle flakiness from microsecond drift between InitChain and the initial context. This should stabilize time-dependent tests (e.g., auction closing time relations).
Also applies to: 165-165, 169-169
packages/injective-test-tube/src/module/auction.rs (3)
26-28: API surface: adds query_current_auction_basketThe new query is correctly wired to the expected endpoint and types.
41-47: Imports: tidy and minimalConsolidated crate imports and added the missing request type. Looks clean.
87-111: Confirmedget_block_time_secondssignature; cast tou64is required and safeI verified that in
packages/injective-test-tube/src/runner/app.rs,pub fn get_block_time_seconds(&self) -> i64 { … }so the
as u64cast at the call site is not redundant. Since the test harness never produces negative block times, it won’t underflow or panic. No changes are needed here.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/injective-test-tube/src/module/auction.rs (3)
95-102: Avoid unnecessary clones when printingCloning
highest_bid_amountandhighest_biddersolely forprintln!introduces avoidable allocations. Borrow or reference the fields directly.Apply this diff:
- let highest_bid = basket_res.highest_bid_amount.clone(); - let highest_bidder = basket_res.highest_bidder.clone(); - assert_eq!(round, 0, "Round should be 0"); println!( "[check] round={}, closing_time={}, highest_bidder={}, highest_bid_amount={}", - round, closing_time, highest_bidder, highest_bid + round, closing_time, &basket_res.highest_bidder, &basket_res.highest_bid_amount );
128-128: Compute delta from current block time and guard against underflowUsing the initial
block_time_secto compute the jump can be slightly confusing after the prior 1s increase, and in future refactors could risk underflow. Derive the delta from the current block time and usesaturating_subfor safety.Apply this diff:
- app.increase_time(closing_time - block_time_sec + 1); + let now = app.get_block_time_seconds() as u64; + app.increase_time(closing_time.saturating_sub(now) + 1);
134-142: Strengthen assertions: expect exactly one round advance and deterministic next closing timeGiven the time jump crosses the boundary by just over 1 second, asserting exact transitions makes the test more deterministic and future-regression-proof. You can reuse the module’s params to validate the next closing time precisely.
Apply this diff:
- assert!( - basket_response_after_increase.auction_round > round, - "Round should increase" - ); - - assert!( - basket_response_after_increase.auction_closing_time > closing_time, - "Closing time should increase" - ); + let params = auction + .query_auction_params(&QueryAuctionParamsRequest {}) + .expect("query_auction_params should succeed") + .params + .expect("params should be set"); + + let expected_next_closing_time = closing_time + params.auction_period; + + assert_eq!( + basket_response_after_increase.auction_round, + round + 1, + "Round should increment by exactly 1" + ); + + assert_eq!( + basket_response_after_increase.auction_closing_time, + expected_next_closing_time, + "Closing time should advance by exactly one auction period" + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/injective-test-tube/src/module/auction.rs(2 hunks)
🔇 Additional comments (3)
packages/injective-test-tube/src/module/auction.rs (3)
41-41: Import consolidation looks goodBringing
AuctionandInjectiveTestAppin via a single crate import improves readability and avoids duplication.
45-47: Good addition: basket query types imported for coverageIncluding
QueryCurrentAuctionBasketRequest(alongside existing params/last result) is appropriate for the new assertions.
112-127: Solid invariant checks pre- and post small time increaseThe 1-second bump followed by asserting the round and closing_time remain unchanged is precise and guards against premature rollovers.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/unit_test.yml (1)
56-56: Clippy with--lockedis good; add trailing newline to satisfy lint.Yamllint flags “no new line character at the end of file.” Add a final newline to this file to silence the warning. Also consider upgrading to actions/checkout@v4 in a follow-up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.github/workflows/unit_test.yml(3 hunks).gitignore(0 hunks)
💤 Files with no reviewable changes (1)
- .gitignore
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/unit_test.yml
[error] 56-56: no new line character at the end of file
(new-line-at-end-of-file)
⏰ 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). (2)
- GitHub Check: Test Suite
- GitHub Check: Lints
🔇 Additional comments (2)
.github/workflows/unit_test.yml (2)
1-1: Top-level workflow name is correct and clearer—LGTM.Moving the name to the top aligns with common practice and avoids duplication.
29-29: Cargo.lock verified and--lockedis safe to use
ACargo.lockfile exists at the repo root and is tracked by Git, so the--lockedflag in.github/workflows/unit_test.ymlwill succeed. No changes are required.Optional improvement:
- To speed up CI, consider adding a Rust cache step (e.g.,
Swatinem/rust-cache@v2) before running tests.
Summary by CodeRabbit
Bug Fixes
Tests
CI
Chores