Split EnvBase testutils-gated stuff into its own trait#1623
Split EnvBase testutils-gated stuff into its own trait#1623graydon wants to merge 1 commit intostellar:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors test-only functionality from the EnvBase trait into a separate EnvBaseTestUtils trait to better organize code that is only available when the testutils feature is enabled. This addresses issue #1570 by creating a cleaner separation between core environment functionality and test-specific utilities.
Key Changes:
- Created new
EnvBaseTestUtilstrait insoroban-env-common/src/env.rsto hold test-only methods - Moved
escalate_error_to_panicmethod and deprecated hook methods fromEnvBaseto the new trait - Updated implementations in both
HostandGuestto implement the new trait separately
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| soroban-env-common/src/env.rs | Defines new EnvBaseTestUtils trait with test-only methods moved from EnvBase |
| soroban-env-common/src/lib.rs | Exports the new EnvBaseTestUtils trait when testutils feature is enabled |
| soroban-env-host/src/host.rs | Moves escalate_error_to_panic implementation to separate EnvBaseTestUtils impl block |
| soroban-env-guest/src/guest.rs | Moves escalate_error_to_panic implementation to separate EnvBaseTestUtils impl block and adds import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Return true if the environment wants to receive trace calls and and | ||
| /// returns using [`Self::trace_env_call`] and [`Self::trace_env_ret`]. | ||
| #[cfg(feature = "std")] |
There was a problem hiding this comment.
The cfg's around these functions gated by a feature could potentially create the same problem in the future.
|
Because this change modifies an exported trait, it's a breaking change, so it's going to need to target v26. Is that the intent? |
Superficial fix for #1570 lazily composed by claude. Whether this is actually a thing we want remains to be seen! Asking followup in the bug.