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

Single DeployHelper for Deployment and Tests #36

Merged
merged 15 commits into from
Apr 9, 2024

Conversation

jdubpark
Copy link

@jdubpark jdubpark commented Apr 5, 2024

Context

Currently, the codebase contains two separate main deployment helpers, one inherited by all tests and another for the actual protocol deployment. Keeping these two files synced can lead to disparity, thus this PR intends to merge the two together to create a unified deploy helper.

Changes

All changes were made in the script & test folders

  • Tests use the new BaseTest, which inherits from the deploy script in the script folder.
  • Modify some tests to clarify function callers (using pranks).
  • Minor lint fixes for state variable visibility.
  • Added Mock Royalty Policy LAP for License-specific tests to skip royalty payment logic.
  • Prepare codebase structure to remove some mocks in the next PR.
  • Remove unused test file(s).
  • Remove fork test in GH action.

Misc.

After this PR, we intend to

  • Remove some mocks in tests, as some "unit" tests are in fact mini-integration tests (due to the high interweaving of our protocol contracts).

@jdubpark jdubpark changed the title Single Deploy Helper for Less Maintenance Single DeployHelper for Deployment and Tests Apr 5, 2024
@jdubpark jdubpark marked this pull request as ready for review April 8, 2024 02:21
@LeoHChen LeoHChen merged commit ec35ce0 into storyprotocol:main Apr 9, 2024
2 checks passed
kingster-will pushed a commit to kingster-will/protocol-core-v1-dev that referenced this pull request Jan 31, 2025
irfand29 pushed a commit to irfand29/protocol-core-v1 that referenced this pull request Feb 8, 2025
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.

4 participants