-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Open
Description
Description
Currently, our integration test framework modifies global build constants to create a testing environment that mimics mainnet conditions (PR #12932). This approach causes several problems:
// From PR #12932
buildconstants.UpgradeAssemblyHeight = -1
buildconstants.InitialFilReserved = types.MustParseFIL("700000000 FIL").Int
buildconstants.UpgradeTeepInitialFilReserved = types.MustParseFIL("1400000000 FIL").IntProblems with current approach
- Hidden side effects: Modifying global constants creates unseen side effects that can affect other parts of the system.
- Prevents parallel testing: These global changes make it impossible to run tests in parallel (
t.Parallel()). - Potential for runtime issues: Any code that reads these variables before node construction may see inconsistent values.
- Maintainability concerns: Special-casing test environments through global mutation is difficult to track and maintain.
Proposed solution
We need a more elegant configuration approach for tests:
- Create a proper gereralised "config" object pattern that encapsulates network parameters
- Construct this within the node builder and feed it where it's used via the DI framework
- Allow tests to create instances of this config with custom values
- Pass these config objects to node construction rather than relying on globals
- Ensure configuration is localised to specific test instances to enable parallel testing
This would replace the current pattern of directly modifyingbuildconstantsvalues in test setup.
Related discussions
This issue follows from the discussion in PR #12932 where @Stebalien noted:
"The wider issue is that we should be using these constants to construct some form of 'config' object which we should be using internally."
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
Status
📌 Triage