Skip to content

Move with_custom_state to infra & add tests#4859

Open
galke7 wants to merge 1 commit intoethereum:masterfrom
galke7:fix/move-custom-context
Open

Move with_custom_state to infra & add tests#4859
galke7 wants to merge 1 commit intoethereum:masterfrom
galke7:fix/move-custom-context

Conversation

@galke7
Copy link
Copy Markdown

@galke7 galke7 commented Jan 24, 2026

Description

This PR refactors the testing infrastructure by moving the with_custom_state decorator from the core spec context (tests/core/pyspec/eth2spec/test/context.py) to the infrastructure context (tests/infra/context.py).

Why this change is necessary:
The with_custom_state function is a testing utility, not a core specification rule. Keeping it in the core module risked creating circular dependencies as the test suite grows. Moving it to tests/infra ensures better modularity and allows other test modules to import it without conflict.

Additionally, this PR adds a new test file tests/infra/test_custom_state.py to verify that the decorator correctly injects custom state balances and parameters, ensuring the utility works as expected in isolation.

Checklist

  • Update documentation (if applicable)
  • Add tests for new functionality (if applicable)
  • Run make lint to check formatting
  • Run make test to check tests

Relations

Fixes #4669

@galke7 galke7 changed the title Refactor: Move with_custom_state to infra and add tests Refactor: Move with_custom_state to infra & add tests Jan 24, 2026
@leolara leolara self-requested a review January 25, 2026 08:37

# --- The Test Case ---

def test_custom_state_decorator():
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should more test cases, specially that test both parameters with different values. Perhaps it is better if the test function returns what you are trying to assert over.

@jtraglia jtraglia added the testing CI, actions, tests, testing infra label Jan 26, 2026
@galke7 galke7 force-pushed the fix/move-custom-context branch from f768588 to 69c32ce Compare January 28, 2026 02:45
@galke7
Copy link
Copy Markdown
Author

galke7 commented Jan 28, 2026

Thanks for the guidance.
I've updated the PR to address your points:

  • Refactored to Return Values: The test function now returns the state balance directly for assertion, removing the side-effect list.

  • Increased Coverage: I implemented a table-driven test (matrix) that verifies three scenarios:
    Standard values (32 ETH threshold)
    Custom values (16 ETH threshold)
    Boundary check (Balance == Threshold)

@galke7 galke7 force-pushed the fix/move-custom-context branch from 69c32ce to dbce783 Compare January 29, 2026 21:41
@jtraglia jtraglia changed the title Refactor: Move with_custom_state to infra & add tests Move with_custom_state to infra & add tests Jan 30, 2026
@jtraglia jtraglia changed the title Move with_custom_state to infra & add tests Move with_custom_state to infra & add tests Jan 30, 2026
@galke7 galke7 requested a review from leolara February 2, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit tests with_custom_state

3 participants