Skip to content
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

Try to lighten the test process. #5

Merged
merged 31 commits into from
Jul 30, 2024
Merged

Conversation

clement-ux
Copy link
Contributor

@clement-ux clement-ux commented Jul 19, 2024

This is a PoC for a new testing structure.

Objectif

  • Lighten the test and deployment process.
  • Propose a simple way to run tasks (without Relayer usage).
  • Make tests run faster.
  • Propose a new testing structure.

Main points

  • Decoupling testing and deployment files: Testing and deploying will be completely separated from each other. This allows a faster running time for tests, a simpler deployment script, and more composability for tests.
  • The new test structure allows better composability and removes the complexity of the "fork test", as the choice of "fork" will be done directly on the test, using vm.createSelectFork(). I.e. to run all tests, (fork and not fork), forge t is enough.
  • Introduction of a Makefile, which contains all the useful commands to run tests, deploy contracts (simulation and real deployment), run tasks (doesn't work with OZ Relayer), and coverage.

Advices

  • Instead of providing directly an Alchemy/Infura/Llama RPC on $PROVIDER_URL, run anvil -f "RPC_URL" --fork-block-number "BLOCK_NUMBER" and set PROVIDER_URL=http://localhost:8545 on .env. This should speedup the process.
  • The current RPC should be the latest tenderly virtual testnet. OETH ARM 2. Otherwise, test for requestWithdraw() and claimWithdraw() will fail.

Test file structure

The test structure should be organized like the following:

  • Base.sol: This contract is the base for ALL the tests in the repo (fork on not, invariant, fuzzing etc.). It should inherit (directly or indirectly) from the Test.sol from Forge. It should be abstract. See contract description.
  • utils folders containing utils stuff (Addresses.sol lib, Math.sol lib etc.), that can be shared across all tests!
  • fork folder (if any fork test)
  • unit folder (if any unit test)
  • integration folder (if any integration test)
  • invariant folder (if any invariant test)

Each test folder (fork, unit, or integration) should have the same structure inside:

  • concrete folder: To test simple actions.
    • functionName.t.sol: One test file (ending with .t) per function teste (in the best case). This is here and only here that tests should happen. All test files should inherit (directly or indirectly) from Shared.sol.
  • fuzz folder: To test complex math action:
    • functionName.t.sol: Same as above.
  • shared folder
    • Shared.sol: This contract should inherit (directly or indirectly) from Base.sol. It is in this contract that all contracts are deployed, addresses are generated (see contract description).
  • utils folder:
    • Modifiers.sol (used to simplify test setup)
    • MockCall.sol (used to mock call instead of creating a mock contract)
    • Helpers.sol (all that can help, like override forge function).
    • etc.
      Note: invariant testing architecture is a bit different.

This test structure is completely inspired from https://github.com/sablier-labs/v2-core/tree/main/test.

Comment on lines +67 to +75
// Deploy OEthARM implementation.
address implementation = address(new OEthARM());
vm.label(implementation, "OETH ARM IMPLEMENTATION");

// Initialize Proxy with OEthARM implementation.
proxy.initialize(implementation, Mainnet.TIMELOCK, "");

// Set the Proxy as the OEthARM.
oethARM = OEthARM(address(proxy));
Copy link
Collaborator

Choose a reason for hiding this comment

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

For fork tests, I'd prefer if we use the same deploy framework as mainnet (So DeployManager). This would ensure that the contracts and config being tested on fork would be same as on mainnet

@clement-ux clement-ux marked this pull request as ready for review July 22, 2024 13:52
@naddison36 naddison36 merged commit ea16ea9 into main Jul 30, 2024
2 checks passed
@clement-ux clement-ux deleted the clement/ligther-test-process branch August 5, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants