This repository has been archived by the owner on Apr 30, 2024. It is now read-only.
generated from storyprotocol/solidity-template
-
Notifications
You must be signed in to change notification settings - Fork 70
New Unified Test Framework #90
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LeoHChen
reviewed
Feb 13, 2024
Ramarti
approved these changes
Feb 13, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jdubpark, an unified test framework is a huge improvement. Always easier to start with one that that move to one, great effort.
Not for this PR, but onwards to mainnet we have to try and minimize mocking to maybe the tokens and external contracts IMO. Mocks have allowed us to work in parallel, but they are more code to maintain and make the tests arguably weaker. It will be easier with this framework.
test/foundry/modules/licensing/UMLPolicyFramework.multi-parent.sol
Outdated
Show resolved
Hide resolved
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduction
This PR introduces a new, unified testing framework, namely
BaseTest.sol
andDeployHelper.sol
intest/foundry/utils
. The former contract is to be inherited by all test contracts in unit and integration tests, where it will provide access to all contracts available on our core repo.Through the
BaseTest
, a test contract can select which contracts to deploy as real contracts and which contracts to deploy as mock contracts. The framework lazy loads the deployments, so any contracts irrelevant to a test contract will not be deployed.NOTE 1: when deploying contracts in a test, we must fully understand the dependencies of each deployed contract so that we deploy all necessary contracts. However, we don't have to deploy all dependencies (use mock instead).
NOTE 2: if facing a test error, check the full trace logs to see what errors or console2.logs are emitted.
BaseTest
andDeployHelper
have logs to indicate both the deployment status (real/mock) and post-deployment status.Test
setUp
FlowThe framework flow is:
BaseTest
and callBaseTest.setUp()
buildDeploy$$$Condition
functions with correspondingDeploy$$$Condition
structs, where "$$$" is a group of relevant contracts, such as registries and modules.deployConditionally()
after building all deploy conditions.postDeploymentSetup()
to set up the deployed contracts, such as initializing module addresses.Example 1 (straightforward)
In this example, we deploy the
registrationModule
,disputeModule
,ipResolver
,arbitrationPolicySP
, androyaltyPolicyLS
. The post-deployment setup will set configs for these contracts, if applicable.As a dependency,
registrationModule
requires a valid, deployedipResolver
(currently no mock).Example 2 (less straightforward)
In this example, we require three mock contracts:
accessController
,licensingModule
, androyaltyModule
. ThegetXXX
methods retrieve existing contracts (if deployed via condition = true) OR deploy mock contracts.Caveat
As Example 2 shows, currently it's not possible to auto-deploy mock contracts lazily. As a future solution, instead of booleans, we should provide three options (enum) in deploy conditions: auto, mock, and real. With these options, we won't call
getXXX
to get a mock contract if there's no real contract deployed — theauto
option will lazily deploy a mock contract.Caution
While this framework makes deployment easier, it requires manual fixes when new contracts are introduced in existing groups, or if existing contracts get new dependencies. While this is mostly an easy intervention, dependency can require more custom logic as contracts can be deployed after the dependent contract.
Misc. Changes