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

refactor: [v0.8-develop] account test base #44

Merged
merged 2 commits into from
Apr 17, 2024

Conversation

adam-alchemy
Copy link
Contributor

Motivation

All tests in test/account used a similar setup sequence - deploying the EntryPoint, deploying SingleOwnerPlugin, setting up the account and owners.

Solution

To reduce repeated boilerplate code, pull out this test setup to a single test base called AccountTestBase, and make the aforementioned tests inherit from this.

To do this, I had to slightly refactor UpgradeableModularAccount.t.sol - previously, it used account1 to refer to the un-deployed account counterfactual and account2 to refer to the deployed account, but to use the test base, I had to switch these.

Also had to fix the mocks used by the execute from plugin tests, because they previously hardcoded deployment addresses coming from a nonce (create1), but the new abstract test base changed them. Refactored to correctly parameterize the addresses.

@adam-alchemy adam-alchemy changed the title refactor: account test base refactor: [v0.8-develop] account test base Apr 16, 2024
Base automatically changed from adam/6900-iteration-setup to v0.8-develop April 16, 2024 17:41
Copy link
Collaborator

@howydev howydev left a comment

Choose a reason for hiding this comment

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

LGTM

@adam-alchemy adam-alchemy merged commit a66dcd8 into v0.8-develop Apr 17, 2024
3 checks passed
@adam-alchemy adam-alchemy deleted the adam/test-base branch April 17, 2024 19:30
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.

None yet

3 participants