Skip to content

Improve testing environment configuration to avoid global constant modification #12963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rvagg opened this issue Mar 19, 2025 · 0 comments
Open

Comments

@rvagg
Copy link
Member

rvagg commented Mar 19, 2025

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").Int

Problems with current approach

  1. Hidden side effects: Modifying global constants creates unseen side effects that can affect other parts of the system.
  2. Prevents parallel testing: These global changes make it impossible to run tests in parallel (t.Parallel()).
  3. Potential for runtime issues: Any code that reads these variables before node construction may see inconsistent values.
  4. 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:

  1. Create a proper gereralised "config" object pattern that encapsulates network parameters
  2. Construct this within the node builder and feed it where it's used via the DI framework
  3. Allow tests to create instances of this config with custom values
  4. Pass these config objects to node construction rather than relying on globals
  5. Ensure configuration is localised to specific test instances to enable parallel testing
    This would replace the current pattern of directly modifying buildconstants values 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."

@github-project-automation github-project-automation bot moved this to 📌 Triage in FilOz Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📌 Triage
Development

No branches or pull requests

1 participant