test: cover pending.go helpers and legacy migration

[CaelRowley]

Overview

Follow-up to #3073 adding unit coverage for block/internal/executing/pending.go.

The existing restart tests in executor_restart_test.go cover the end-to-end happy path but leave migrateLegacyPendingBlock and several error branches untested. These gaps are now covered in pending_test.go:

• migrateLegacyPendingBlock: happy-path migration, no-op, and signed-candidate guard
• getPendingBlock: corrupt-state and empty-store branches
• savePendingBlock + deletePendingBlock: round-trip verification

No production code changes.

Testing

go test ./block/internal/executing/ -run 'TestMigrateLegacyPendingBlock|TestGetPendingBlock|TestDeletePendingBlock' -v -count=1

Summary by CodeRabbit

• Tests
  • Added comprehensive test coverage for pending block handling, including legacy block migration, state consistency validation, and verification of proper error handling and data lifecycle operations.

[coderabbitai]

Warning

Rate limit exceeded

@CaelRowley has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 53 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 53 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b9410f02-c628-4e71-840c-77e0efc09fbc

📥 Commits

Reviewing files that changed from the base of the PR and between 744c8e5 and 348aa72.

📒 Files selected for processing (1)

• block/internal/executing/pending_test.go

📝 Walkthrough

Walkthrough

A new test file for pending-block functionality has been added, introducing helper constructors (buildHeaderAndData, saveLegacyBlock) and four test cases validating block migration from legacy keys, state consistency checks, empty state handling, and block deletion operations.

Changes

Cohort / File(s)	Summary
Pending Block Tests block/internal/executing/pending_test.go	Added test helpers and four test cases covering legacy block migration, state corruption detection, empty pending block retrieval, and save/read/delete operations for pending blocks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Whiskers twitch with testing glee,
Legacy blocks set newly free,
Pending states both checked and cleared,
Migrations safe, no bugs we feared!
Hops of joy for tests so bright,
Making code behave just right! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change—adding test coverage for pending.go helpers and legacy migration.
Description check	✅ Passed	The description provides comprehensive context, explicitly references the related PR, details all test scenarios covered, clarifies the scope of testing, and includes a command to verify the changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

block/internal/executing/pending_test.go (1)

44-44: Use a fixed timestamp to keep tests deterministic.

Line 44 uses time.Now().UnixNano(), which makes test data time-dependent. Prefer a constant timestamp in test helpers.

Proposed change

-			BaseHeader: types.BaseHeader{
+			BaseHeader: types.BaseHeader{
 				ChainID: gen.ChainID,
 				Height:  height,
-				Time:    uint64(time.Now().UnixNano()),
+				Time:    uint64(1_700_000_000_000_000_000), // fixed test timestamp
 			},

As per coding guidelines, "`**/*_test.go`: Ensure tests are deterministic".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@block/internal/executing/pending_test.go` at line 44, Replace the
non-deterministic timestamp used for test data (uint64(time.Now().UnixNano()) in
pending_test.go) with a fixed constant to make tests deterministic: create a
package-level const or var like testTimestamp (or a local fixedTS) and use that
for the Time field in the test helper/fixture that builds the pending object.
Update any test helpers or struct initializers that reference Time in
pending_test.go (the initializer where Time is set) to use the fixed timestamp
instead of time.Now().

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@block/internal/executing/pending_test.go`:
- Around line 103-107: The tests currently accept any error from
fx.MemStore.GetBlockData and fx.MemStore.GetSignature which can mask backend
failures; update the assertions to require the specific sentinel error
ds.ErrNotFound instead of a generic assert.Error. Locate the calls to
fx.MemStore.GetBlockData(t.Context(), 1) and
fx.MemStore.GetSignature(t.Context(), 1) (and the similar checks at lines
112-113) and replace the generic error assertions with an explicit
equality/assertion against ds.ErrNotFound so the test fails only when a key is
unexpectedly present rather than on unrelated errors.

---

Nitpick comments:
In `@block/internal/executing/pending_test.go`:
- Line 44: Replace the non-deterministic timestamp used for test data
(uint64(time.Now().UnixNano()) in pending_test.go) with a fixed constant to make
tests deterministic: create a package-level const or var like testTimestamp (or
a local fixedTS) and use that for the Time field in the test helper/fixture that
builds the pending object. Update any test helpers or struct initializers that
reference Time in pending_test.go (the initializer where Time is set) to use the
fixed timestamp instead of time.Now().

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a2170101-548a-4380-93cd-c11a97347c48

📥 Commits

Reviewing files that changed from the base of the PR and between 3efcf93 and 744c8e5.

📒 Files selected for processing (1)

• block/internal/executing/pending_test.go

[CaelRowley]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[julienrbrt]

Thanks! but we won't merge this. migrateLegacyPendingBlock will get deleted in #2795 in the next version.