Skip to content

Conversation

@RodrigoVillar
Copy link
Contributor

@RodrigoVillar RodrigoVillar commented Dec 2, 2025

Why this should be merged

Currently, TestExportRange() is a multi-step process (which doesn't test anything) that's housed inside a unit test - it would be more idiomatic if TestExportRange() was it's own executable. This PR accomplishes this by making TestExportRange() into its own executable separate from BenchmarkReexecuteRange(), eliminating the need to host it in a unit test.

Converting TestExportRange() into an executable also allows us to remove the go bench logic from BenchmarkReexecuteRange() in a future PR (#4640) as there can only be one main() function per package.

This PR follows #4638 and precedes #4640.

How this works

Moves the export block range functionality to its own package blockexport + updates Taskfile.yml to use this new package.

How this was tested

CI + ran task export-cchain-block-range locally

Need to be documented in RELEASES.md?

No

Base automatically changed from rodrigo/export-create-block-chan to master December 3, 2025 22:11
@RodrigoVillar RodrigoVillar self-assigned this Dec 3, 2025
@RodrigoVillar RodrigoVillar marked this pull request as ready for review December 3, 2025 23:50
Copilot AI review requested due to automatic review settings December 3, 2025 23:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the block export functionality by converting TestExportRange() from a test function into a standalone executable. This change makes the code more idiomatic and sets the stage for future refactoring work by separating concerns between testing and utility executables.

Key Changes:

  • Created new blockexport package with standalone executable for block export functionality
  • Modified CreateBlockChanFromLevelDB to accept generic cleanup handler instead of testing.TB
  • Updated Taskfile to use go run instead of go test for block export operations

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/reexecute/db.go Updated function signature to accept generic require.TestingT and cleanup handler for broader reusability
tests/reexecute/c/vm_reexecute_test.go Removed test-specific export function and related flags, updated caller to pass cleanup handler
tests/reexecute/blockexport/main.go Added new standalone executable with main function for block export functionality
Taskfile.yml Changed command from go test to go run to execute new blockexport executable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@aaronbuchwald aaronbuchwald left a comment

Choose a reason for hiding this comment

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

LGTM - one minor nit

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants